Skip to content

Update Poetry Version #1290

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.

Sign up for GitHub

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

Jump to bottom
Merged
BryonLewis merged 4 commits into from
Sep 8, 2022
Merged

Update Poetry Version #1290

BryonLewis merged 4 commits into from
Sep 8, 2022

Conversation

@BryonLewis
Copy link
Collaborator

@BryonLewis BryonLewis commented Sep 2, 2022 •
edited
Loading

  • Older version of poetry we were using (1.1.2) was causing some build errors.
  • Updated to the latest version (1.2.0) but it had different behaviors when determining that the currently used python virutalenv was active.
  • From the Poetry Environment Management Docs it should recognize that it is inside of a venv and not create a new one. It did this properly for the 1.1.2 version but not for 1.2.0.
  • To force it to recognize the the virtualenv I had to set the Environment variable manually.
  • Additionally, on the worker container, I needed to make sure it did a full copy of the files and didn't use symbolic links for the venv. The final container for the worker is based off a VIAME GPU algorithms container so it isn't guaranteed to have the same symlink python pointers. This works for the girder/web container because the builder and the final container both use the same base python3.8.
  • Also modified the pyproject.toml to support a more updated structure.

@waxlamp
Copy link
Member

waxlamp commented Sep 2, 2022

Thanks for this fix, @BryonLewis. Has the Poetry bug been reported upstream? If not, can you do so?

@BryonLewis
Copy link
Collaborator Author

BryonLewis commented Sep 5, 2022

I haven't spent significant time searching within the Poetry Github to see if it was a regression, intended change from jumping from 1.1.2 past 1.1.15 and all the way to 1.2.0 or if there is duplicate issue already (poetry has 991 backlogged issues). I imagine we were taking advantage of some quirk which allowed it to identify virtualenvs indirectly. Explicitly stating them allowed us to update to the latest version. I ran the integration tests and they all worked (except one related to updating the addons).

@BryonLewis BryonLewis requested a review from marySalvi September 5, 2022 17:55
marySalvi
marySalvi approved these changes Sep 7, 2022
View reviewed changes
Copy link
Collaborator

@marySalvi marySalvi left a comment

Choose a reason for hiding this comment

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

LGTM

@BryonLewis BryonLewis merged commit 4206250 into main Sep 8, 2022
@BryonLewis BryonLewis deleted the fix-poetry branch September 8, 2022 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marySalvi marySalvi marySalvi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@BryonLewis @waxlamp @marySalvi