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

Use poetry for dependency management #394

Merged
merged 17 commits into from
Apr 6, 2023
Merged

Use poetry for dependency management #394

merged 17 commits into from
Apr 6, 2023

Conversation

grahamalama
Copy link
Contributor

@grahamalama grahamalama commented Mar 22, 2023

Migrate from pip-tools to Poetry for dependency management

  • format toml
  • incorporate into CircleCI or move linting / tests to GHA
    • it might be good to move everything onto GHA in a separate PR. Or not.
  • Fine-tune poetry installation, commands, dependency groups
  • Update README
  • Update CHANGELOG

.readthedocs.yaml Outdated Show resolved Hide resolved
@leplatrem
Copy link
Contributor

YES 🤩

.circleci/config.yml Show resolved Hide resolved
.readthedocs.yaml Outdated Show resolved Hide resolved
@grahamalama grahamalama marked this pull request as ready for review March 24, 2023 16:35
@grahamalama grahamalama requested a review from a team as a code owner March 24, 2023 16:35
@leplatrem leplatrem self-assigned this Mar 27, 2023
@@ -233,17 +233,14 @@ jobs:

unit_test:
docker:
- image: python:3.10.5-bullseye@sha256:dac61c6d3e7ac6deb2926dd96d38090dcba0cb1cf9196ccc5740f25ebe449f50
- image: cimg/python:3.11.2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a tag cimg/python:3.11? ie. do we want to tag minor here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is, but I thought it'd be good to make sure we use the same patch version that we use in our Dockerfile. Or is that being too careful?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being careful is good :) I was thinking that we could have a test that verifies that both versions are in sync. I doubt that dependabot will upgrade this line. Will it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't think it will update this line.

We're actually in an interesting situation now where there's a python:3.11.3 published, but there isn't a cimg/python:3.11.3 yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.readthedocs.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@leplatrem leplatrem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your efforts on this one 👏


# though we have kinto-remote-settings specified as a dependency in
# pyproject.toml, we have it configured to install in editable mode for local
# development. For building the container, we only install the "main"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in editable mode for local development

because we use --no-root in make install? and develop = true in pyproject.toml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think develop = true is more important than --no-root in this context

@grahamalama grahamalama merged commit 03caa43 into main Apr 6, 2023
@grahamalama grahamalama deleted the use-poetry branch April 6, 2023 20:24
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 this pull request may close these issues.

2 participants