-
Notifications
You must be signed in to change notification settings - Fork 109
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
Datapusher for inventory-classic and inventory-next #2073
Conversation
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.
Looks great!
FWIW, datapusher is really a completely separate application from inventory. And by that logic it could/should be it's own role, playbook, its own OS user, its own logs dir, etc. This PR will make it much easier to separate it out into it's own role when we're ready to pick up that work. When we start moving datapusher to cloud.gov, that would be a good time to do that since it would be deployed separately from inventory and moving to a separate role would keep parity between cloud.gov and BSP.
datapusher_build_pkg_repo: https://github.com/GSA/datapusher | ||
datapusher_build_pkg_requirements: requirements-freeze.txt | ||
datapusher_gunicorn_error_log: "{{ inventory_log_dir }}/datapusher_gunicorn.log" | ||
datapusher_next_repo: https://github.com/ckan/datapusher.git |
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.
👏
requirements: requirements-freeze.txt | ||
datapusher_build_pkg_name: datapusher | ||
datapusher_build_pkg_repo: https://github.com/GSA/datapusher | ||
datapusher_build_pkg_requirements: requirements-freeze.txt |
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.
Good variable names.
@@ -14,14 +14,6 @@ platforms: | |||
image: ubuntu:trusty | |||
groups: | |||
- trusty | |||
- name: ckan-inventory-app-bionic | |||
image: ubuntu:bionic |
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 wouldn't have removed this but it's correct that this configuration will never be run on bionic. I'd prefer to test all roles on both trusty and bionic where possible. That way we don't have to think about is this role supported on trusty vs bionic (less thinking or knowledge about the code is better).
Are you seeing that this scenario no longer works on bionic?
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 think I was trying to be too efficient on tests, and have different scenarios for classic and next. Instead, I'm going to slightly refactor to pull back in bionic and only have next specific tests on the inventory-next scenario...
with_items: | ||
- etc/ckan/datapusher.wsgi | ||
- etc/ckan/datapusher_settings.py | ||
- etc/supervisor/conf.d/gunicorn-datapusher.conf |
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.
Fwiw, I would just name this "datapusher" since gunicorn is just an implementation detail.
@@ -0,0 +1,10 @@ | |||
[program:gunicorn-datapusher] | |||
command={{ datapusher_virtual_env }}/bin/gunicorn -b 127.0.0.1:6000 wsgi:app |
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.
Similar to inventory, I think we want to use an async worker type. The default is probably fine for now though.
….yml Co-authored-by: Aaron D Borden <[email protected]>
…m/GSA/datagov-deploy into feature/inventory-next-datapusher
This refactors how datapusher is deployed, moving into it's own tasks file for ticket #2037.
Since datapusher is configured differently for inventory-next (using gunicorn, using upstream, etc), I split them into completely different workflows.
Since some of the deployment verification was different for datapusher, I moved inventory-next/bionic testing into it's own molecule scenario (similar to catalog-next). Both systems passed locally.
Needed to pull in Saml2 updates for inventory to validate this was working as expected and to be able to test on sandbox.
All testing has passed, this is ready to merge.