-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP - 13 dockerize #14
base: master
Are you sure you want to change the base?
Conversation
poikilotherm
commented
Dec 5, 2019
- Closes Dockerize for usage with K8s and other scenarios #13
subpath = os.getenv('DATAVERSE_SERVICE_SUBPATH', '') | ||
base_url = os.getenv('BASE_URL', proto+'://'+host+':'+port+subpath) | ||
|
||
api_token = os.getenv('API_TOKEN', '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned that the change to api_token
will break this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it will! But Ansible allows for a regex, so maybe we can change that one?
I talked about this briefly at standup. I think it's ok to merge but we'd want to follow up basically immediately with a change to the regex at https://github.com/IQSS/dataverse-ansible/blob/1449311f2bb5a260b08c001ab51f85c762b5a510/tasks/sampledata.yml#L52-L56 I'll coordinate with @donsizemore about this. |
Service reminder: please don't merge yet, I would like to include the Jenkins pipeline in this PR, having container image ready to go. |
@poikilotherm oh, in that case, back to "community dev" it goes. 😄 I just created IQSS/dataverse-ansible#138 to track how we'll have to make a change after we merge this pull request. |
@poikilotherm, @pdurbin, what is the status of this issue? We would like to have all data samples ready for Jenkins pipeline integrated with Selenium tests. |
The only thing missing is the webhook from this project to Jenkins to build and deploy to DockerHub. Everything else should be already working. |
We also need to coordinate with @donsizemore as a mentioned at #14 (comment) because we don't want the "sample data" stuff to break in dataverse-ansible. |
5ecc84d
to
8d13e95
Compare
8d13e95
to
4ccd3d5
Compare
@4tikhonov I just added the necessary bit for Jenkins and now @dataversebot is happily pushing things to Docker Hub, see also Jenkins. We should fix IQSS/dataverse-ansible now before merging this PR so no one gets 🔫 . |
Sounds great, @poikilotherm! We'll test it now. |
Alright before we merge this, I might add another script that automates setting the configuration to retrieve the API key, fetches the key, loads the data and deactivates the config setting again. That way it's even easier to use in a K8s job 😉 We can reuse our scripts by using Python subprocess. |
@poikilotherm, good idea, can you make this script fully customizable with DATAVERSE_URL, DATAVERSE_TOKEN and GitHub to sample data repository as a parameters? We need something to wipe the database/SOLR quickly and run the same tests again and again. |
@4tikhonov I dunno if it makes sense to enable retrieval from a different GH repo, as I package sample data from this repo with the container image. The other vars are more or less already present. To what extent or granularity do you want to be able to wipe? Simplest option is drop deployments... This is likely beyond this repository and script scope... |
If it helps, I have "destroy" script that destroys all the datasets and then deletes all the dataverses: https://github.com/IQSS/dataverse-sample-data/blob/baf56de2a9b8b230168d6e6e1758a10201e0ed5e/destroy_all_dvobjects.py |
@pdurbin I appreciate your not wanting to break dataverse-ansible but the |
@donsizemore sounds good. You hitting "approve" put this in QA on the board but I noticed there are merge conflicts. @poikilotherm can you please resolve them? Also, are you still interested in this? It's been a while since we talked about it. |
@poikilotherm do you still want this? As a reminder, there are merge conflicts. Thanks. |
I just gave this old PR a shout out here: |