-
Notifications
You must be signed in to change notification settings - Fork 18
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
Custom pack deps script #101
Custom pack deps script #101
Conversation
But only if executable. Allows pack to customize testing env.
I'd like one more review on this before we merge it. I just noticed something weird: no pack has a |
For background: This PR makes the git clone -b master git://github.com/stackstorm-exchange/ci.git ~/ci
~/ci/.circle/dependencies ; ~/ci/.circle/exit_on_py3_checks $? The next step actually runs the pack tests. For python3, that is: The alternative to including this would be have each pack that needs this edit |
Right. I don't remember where, but I think some packs already did that, for instance installed OS-level dependencies by modifying CircleCI config. I think that's OK approach, but may come with a price you mentioned. |
The feature is "advertised" by adding it by default to new packs. And, for all packs that are currently editing circle config, we could migrate them to use this script. I'm ready/willing to prepare/push PRs for all the exchange packs to drop python2.7 support: StackStorm/community#40 (comment) Either as part of that PR, or a separate one if we can stand the noise of multiple PRs across all exchange packs, that standardizes
|
This is where I'm adding the script by default during pack bootstrap: |
Even with the added magic script For instance, I see a point if users might need to add an external service available for them for some integration test scenarios. This could be done in a native and consistent to CircleCI way by defining new Docker images which is OK and not always possible with the script: build_and_test_python36:
docker:
- image: circleci/python:3.6
- image: rabbitmq:3
- image: mongo:3.4
- image: service1 # added
- image: service2 # added |
This is the conversation (view the thread) that sent me down this path: Jacob Floyd Jan 20th at 23:37
blag 8 days ago
Jacob Floyd 8 days ago
blag 8 days ago
Jacob Floyd 8 days ago
blag 8 days ago
|
@armab Those are good points. If given a choice between the two options of:
then I would choose option 2. I think the original intent behind exchange-incubator was to keep the CircleCI configs as consistent across packs as possible. While some packs may need to customize them, every change makes it more difficult for us to make the occasional sweeping changes across all packs, like removing the Python 2.7 test workflow, or add or updating a Python 3.x workflow in the future. I think adding support for a hook that is outside of the CircleCI config is a win for us. |
@armab does have a valid point wrt adding Dockerizable/Dockerized service to the config. To a certain extent, customizing CircleCI configs a little bit will be unavoidable. But adding a mechanism to keep some customizations out of that file is still beneficial. |
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.
Thanks for the pointers. Yeah, it might be a good hook addition indeed, but we can't force users to use only that mechanism.
Side Note: when removing Python2 job from all the CircleCI configs would be nice to rely on a smarter parsing primitives, rather then just pushing all CircleCI configs to one format.
Ooh. Two 👍 reviews. Could someone merge this please? |
This is an example PR for what I'd like to do to all exchange packs after this is merged: |
Good news: no migrations are required to use this new script in any of the packs. I just looked at the circleci config in all the packs to:
The zabbix pack is the only pack that has significant customization of the circle-ci config. Happily, the zabbix pack only adds some jobs, it doesn't modify the default jobs, so dropping the python2.7 test job is a straight forward merge. The only other notable difference in the packs' circleci config was the weekly schedule: some on Saturday, some on Sunday. (And the test packs' config is significantly out-of-date). |
Instead we want people to use tests/setup_testing_env.sh if possible so that Exchange maintenance is as quick as possible.
I added this warning to
|
Sorry for the commit reference spam in here. Note to self. Do not use URLs in commit messages when working on exchange-wide changes. |
Can we merge this? |
I need some additional setup to run tests in the StackStorm-Exchange/stackstorm-vault#19.
This adds running the custom testing env setup script to the
dependencies
script.It also adds a sample (empty) script and bootstraps new packs with it.