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

Fix #2342, and updates for Poetry 1.2 #2349

Merged
merged 3 commits into from
Sep 2, 2022
Merged

Conversation

glennmatthews
Copy link
Contributor

Closes: #2342

What's Changed

  • Poetry 1.2.0 is released now
    • It has an underdocumented behavior change that poetry install --extras ... followed by poetry install (without any --extras) will result in Poetry removing the previously installed extras.
  • Change our install of Nautobot itself in the dev image to account for this change
  • poetry install --no-dev is deprecated in favor of poetry install --only main, so I've made that change.
  • Add poetry cache clear --all PyPI to the final-dev image build, fixing Docker containers should not include a poetry cache #2342 (with a workaround for a couple of Poetry bugs)

@glennmatthews
Copy link
Contributor Author

Weird, hadolint passes locally. Wonder if I have an old version or something. Will fix when I get time.

@glennmatthews
Copy link
Contributor Author

Actually it's the other way around - CI is using hadolint 2.7 but a locally built dev image has hadolint 2.10. Fails regardless when I run it locally against the actual branch in question 🤦

@bryanculver
Copy link
Member

  • Poetry 1.2.0 is released now

    • It has an underdocumented behavior change that poetry install --extras ... followed by poetry install (without any --extras) will result in Poetry removing the previously installed extras.
  • Change our install of Nautobot itself in the dev image to account for this change

This seems to be a necessary fix as when we ship a new image we'll probably bring in Poetry 1.2 and plugin's development/testing images won't include the necessary dev dependencies at runtime: https://github.com/nautobot/nautobot-plugin-golden-config/blob/develop/development/Dockerfile#L42

@gsnider2195
Copy link
Contributor

Does the poetry installer at https://install.python-poetry.org/ support constraining the version with >=1.2.0 and if so should we add that constraint as part of this change? I know the installer says it will install the latest version so this may not be an issue?

@glennmatthews
Copy link
Contributor Author

It does always install the latest version by default. We could (and probably should) constrain it with the --version argument so that it will use a specific version until we intentionally move to a newer one though.

@bryanculver
Copy link
Member

It does always install the latest version by default. We could (and probably should) constrain it with the --version argument so that it will use a specific version until we intentionally move to a newer one though.

This is how most plugins are installing Poetry: https://github.com/networktocode/gh-action-setup-poetry-environment/blob/main/action.yml

@glennmatthews
Copy link
Contributor Author

This is how most plugins are installing Poetry: https://github.com/networktocode/gh-action-setup-poetry-environment/blob/main/action.yml

Sure, though that's a virtualenv workflow, not a Docker workflow.

I've added an explicit --version 1.2.0 to the install-poetry command so that we can migrate to newer versions on our own schedule rather than immediately upon release.

@@ -137,7 +137,7 @@ RUN apt-get update && \
# https://python-poetry.org/docs/master/#installing-with-the-official-installer
# This also makes it so that Poetry will *not* be included in the "final" image since it's not installed to /usr/local/
RUN curl -sSL https://install.python-poetry.org -o /tmp/install-poetry.py && \
python /tmp/install-poetry.py && \
python /tmp/install-poetry.py --version 1.2.0 && \
Copy link
Member

Choose a reason for hiding this comment

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

Just double checking on the comment above. The installer should install it in a place other than /usr/local but should we enforce this through providing --path or $POETRY_HOME?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--path is for specifying where to find the file to install from, if I'm reading the script right. We could explicitly set POETRY_HOME I suppose but I feel like this is a pretty low risk.

@bryanculver bryanculver mentioned this pull request Sep 2, 2022
6 tasks
@glennmatthews glennmatthews merged commit 834fe95 into develop Sep 2, 2022
@glennmatthews glennmatthews deleted the gfm-issue-2342 branch September 2, 2022 19:53
@whitej6
Copy link
Contributor

whitej6 commented Sep 6, 2022

@bryanculver any chance this can get cherry picked for 1.2 & 1.3 stable releases? Having some "fun" issues with poetry & development containers downstream for 1.2 & 1.3

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.

Docker containers should not include a poetry cache
4 participants