Pin build dependencies and configure dependabot #389
Conversation
tox.ini
Outdated
| deps = | ||
| flake8 | ||
| # https://github.com/JBKahn/flake8-print/pull/30 | ||
| flake8-print>=3.1.0 | ||
| deps = -rlinter-requirements.txt |
There was a problem hiding this comment.
This is where things are going south. Flake8 is pretty generous, compared to flake8 with all the deps in linter-requirements... I vote we expand the ignore fields below to include all the errors from https://github.com/aws/aws-encryption-sdk-python/runs/4541757457?check_suite_focus=true
Can we do something like --ignore D**. That would be awesome...
There was a problem hiding this comment.
Yes, we can: https://flake8.pycqa.org/en/3.1.1/user/options.html#cmdoption-flake8--ignore
So --ignore D would work. In our case, --ignore F811,E203,W503,D
There was a problem hiding this comment.
I am pretty sure D is for documents, and I am okay with having a lower bar for test's documentation.
There was a problem hiding this comment.
Good catch. I'm still curious why this run failed but this one didn't: https://github.com/aws/aws-encryption-sdk-python/runs/4541406129?check_suite_focus=true
They appear to be using the same flake8 versions, but maybe I'm missing a transitive dependency that's different.
There was a problem hiding this comment.
I see, linter-requirements includes flake8-docstrings while the original flake8-tests target did not. I agree that the right path forward is to just ignore all doc stuff for the tests.
.github/dependabot.yml
Outdated
| schedule: | ||
| interval: "daily" | ||
| - package-ecosystem: "pip" | ||
| directory: "/decrypt_oracle" |
There was a problem hiding this comment.
Note, according to dependabot/feedback#113, any .txt file at or one level below directory should be looked at.
|
realized that decrypt_oracle also has it's on tox which defines dependencies. Instead of pinning all of them, bringing decrypt_oracle out of scope for this PR. |
|
Tested on fork that dependebot is able to see all the relevant requirements files. Putting this in draft because there are a couple confounding issues I'm not yet sure how to resolve:
If I can't think of a good resolution to these problems, we may have to scale back to just pinning the linter requirements, as those are the ones that are most likely to break up from updates. |
|
Moved all pinned dependencies to a top level dir, which we point dependabot at. decrypt_oracle remains unaffected, and the freeze-upstream-requirements remains unaffected. I think this is the best balance we can strike right now between stabilizing builds and keeping things simple. |
| # mainline-1.x | ||
| - package-ecosystem: "pip" | ||
| directory: "/dev_requirements" | ||
| schedule: | ||
| interval: "daily" | ||
| target-branch: "mainline-1.x" | ||
|
|
||
| # mainline-2.x | ||
| - package-ecosystem: "pip" | ||
| directory: "/dev_requirements" | ||
| schedule: | ||
| interval: "daily" | ||
| target-branch: "mainline-2.x" |
There was a problem hiding this comment.
Err... what happens when dev_requirements does not yet exist? I assume an error... you may want to remove these until we back port dev_requirements to the other branches.
NVM: Dependabot will probably just not find anything, and move on.
There was a problem hiding this comment.
yeah when I tested this on my fork dependbot just shrugged for whichever targets it didn't find. It will "just work" as soon as those branches are correctly updated.
* chore: Pin build dependencies and configure dependabot * Ignore flake8 document linting on tests * Unpin decrypt_oracle dependencies for now * Pin tox * Isolate pinned dependencies to dev_requirements dir
Note that the deps per tox environment are less granular now, which means that some targets will spend time installing deps they don't necessarily need. We could create requirements files specific to each target, although it might be a bit unweildly. The approach in this PR seems like a decent compromise.
I still need to test that dependabot will be able to find files such as
linter-requirements.txt, as the docs only specifyrequirements.txt, however I've seen other setups that suggest it's not quite that stict.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: