Skip to content
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

Dependabot for more stable CI #488

Closed
adamjstewart opened this issue Mar 30, 2022 · 19 comments · Fixed by #551
Closed

Dependabot for more stable CI #488

adamjstewart opened this issue Mar 30, 2022 · 19 comments · Fixed by #551

Comments

@adamjstewart
Copy link
Collaborator

Our CI has been incredibly unstable lately. Every time a new version of a dependency is released, something in our tests breaks, especially mypy.

Using dependabot, we can pin all of our dependencies to a specific version. The bot will then periodically check for updates and open a PR to update the dependency version. That way, only the version update PR will break, not everyone else's PRs.

Side note: a lot of our deps don't yet have wheels for Python 3.10, or have never had wheels for Windows. I was thinking about switching from pip to conda for all CI. Unfortunately, it looks like dependabot only supports pip, not conda.

@calebrob6
Copy link
Member

I support this! I think we just have to wait for 3.10 wheels...

@calebrob6
Copy link
Member

Now that #457 is merged, we should be able to go ahead here right?

@adamjstewart
Copy link
Collaborator Author

I think so, will have to check exactly what's involved in using dependabot, I've never used it before.

@weiji14
Copy link
Contributor

weiji14 commented Jun 3, 2022

Can I help out with this? I can migrate the dependency list from setup.cfg to the pyproject.toml file, and set things up using poetry which is compatible with dependabot (see https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/about-dependabot-version-updates#supported-repositories-and-ecosystems).

@adamjstewart
Copy link
Collaborator Author

What advantage would there be to using poetry? Wouldn't we still need a separate requirements.txt to store the version pins? We don't want the official list in setup.cfg (or pyproject.toml) to be pinned to a specific version.

@weiji14
Copy link
Contributor

weiji14 commented Jun 3, 2022

I thought we're only pinning the minimum version (which poetry allows for)? Or were you suggesting hard version pins, in which case you could have a poetry.lock file complementing the minimum version pins in pyproject.toml.

@adamjstewart
Copy link
Collaborator Author

Yes, we already pin the minimum (and soon maximum: #544). The point of dependabot is to use hard version pins, but only in CI, so if a new version is released that breaks CI, we can take our time in fixing that instead of having all PRs blocked.

@weiji14
Copy link
Contributor

weiji14 commented Jun 3, 2022

Sure, so the hard version pins can specified in a lockfile, and we can point the CI to install from that lockfile. Maybe easier if I open a PR to illustrate, give me a few minutes. Edit: PR at #552.

@adamjstewart
Copy link
Collaborator Author

So setuptools will require setup.cfg and requirements.txt, and poetry will require pyproject.toml and poetry.lock. So I don't see a reason to couple these changes. If we want to move to poetry, we can in a separate commit. I've been keeping an eye on setuptools/poetry/flit but I haven't seen much reason to move to poetry/flit just yet.

@weiji14
Copy link
Contributor

weiji14 commented Jun 3, 2022

Right, so start using dependabot with the current setuptools regime? I've not used dependabot with requirements.txt before but we can try. Just find poetry to be a bit nicer to use with pinning dependencies (but that's personal preference).

@adamjstewart
Copy link
Collaborator Author

I haven't used poetry before at all 😆

Yeah, I think we should either do things with setuptools and requirements.txt or consider switching to poetry first, but I haven't heard a convincing reason to use poetry yet.

@weiji14
Copy link
Contributor

weiji14 commented Jun 3, 2022

Poetry is like npm for Python, you specify your minimum dependencies in pyproject.toml and there is an associated poetry.lock file that contains exact version pins. Also much easier to specify optional dependencies using poetry.

Whereas with setuptools, the setup.cfg and requirements.txt would be decoupled. Yes, dependabot can update the version pins in the requirements.txt file, but it won't have any knowledge of the pins in setup.cfg (which will need to be manually updated).

@adamjstewart
Copy link
Collaborator Author

Also much easier to specify optional dependencies using poetry.

Can you elaborate on this?

dependabot can update the version pins in the requirements.txt file, but it won't have any knowledge of the pins in setup.cfg

There are pros and cons of this. #544 will add major version pinning for all deps. I would like to have dependabot alert me when a new major version is released, which wouldn't happen if it respected the pins in setup.cfg/pyproject.toml. The con of course is that if you add a new dep you'll have to manually update both files.

@weiji14
Copy link
Contributor

weiji14 commented Jun 3, 2022

Also much easier to specify optional dependencies using poetry.

Can you elaborate on this?

Sure, you can define groups of optional dependencies via 'extras' following https://python-poetry.org/docs/pyproject/#extras. So in the case of torchgeo, the 'datasets'/'style'/'tests' dependencies could be managed nicely.

@adamjstewart
Copy link
Collaborator Author

How is this different than setuptools extras?

@weiji14
Copy link
Contributor

weiji14 commented Jun 3, 2022

How is this different than setuptools extras?

It's probably the same? I don't know the history, but my guess is poetry's extras is inspired or based on setuptools extras. Looking at https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optional-dependencies, I don't see any difference in syntax really for the pyproject.toml part.

image

@adamjstewart
Copy link
Collaborator Author

Okay, so not easier to specify optional dependencies then, just want to clarify.

@adamjstewart
Copy link
Collaborator Author

Whoa, didn't realize setuptools was planning on adding pyproject.toml support. Interesting...

@adamjstewart
Copy link
Collaborator Author

And of course the new pytorch-lightning release breaks our CI as we're discussing this... Will fix that over the weekend if no one else gets to it before me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants