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

Upgrade poetry.lock after updating Docker image to Python 3.10.13 #208

Merged
merged 29 commits into from
Apr 19, 2024

Conversation

Shinnnyshinshin
Copy link
Contributor

@Shinnnyshinshin Shinnnyshinshin commented Apr 17, 2024

What does this PR do?

Reference

  • we are currently seeing failures in the containerize-agent stage of CI/CD : here
#11 47.69 Note: This error originates from the build backend, and is likely not a problem with poetry but with psutil (5.9.8) not supporting PEP 517 builds. You can verify this by running 'pip wheel --use-pep517 "psutil (==5.9.8)"'.
  • Ran poetry update to update the poetry.lock file.
  • The command was run in a python 3.10.13 environment, which matches Docker.

@Shinnnyshinshin Shinnnyshinshin self-assigned this Apr 17, 2024
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.77%. Comparing base (6ddc833) to head (e96bc4a).

❗ Current head e96bc4a differs from pull request most recent head 6839a46. Consider uploading reports for the commit 6839a46 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #208   +/-   ##
=======================================
  Coverage   81.77%   81.77%           
=======================================
  Files          26       26           
  Lines         944      944           
=======================================
  Hits          772      772           
  Misses        172      172           
Flag Coverage Δ
3.10 81.77% <ø> (ø)
3.11 81.77% <ø> (ø)
3.8 81.65% <ø> (ø)
3.9 81.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Shinnnyshinshin Shinnnyshinshin requested review from Kilo59, anthonyburdi, JennyTee and rreinoldsc and removed request for Kilo59 April 17, 2024 23:58
pyproject.toml Outdated
@@ -1,6 +1,6 @@
[tool.poetry]
name = "great_expectations_cloud"
version = "20240417.0"
Copy link
Contributor

@Kilo59 Kilo59 Apr 18, 2024

Choose a reason for hiding this comment

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

This update makes a lot of dependency changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok great idea @Kilo59. I'll follow up here tomorrow.

Copy link
Member

Choose a reason for hiding this comment

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

Agree. @Kilo59 Would it make sense to revert the docker image bump PR as well in the meantime? I'm thinking yes. #205 Then incorporate that change with this in a pre-release as you suggested.

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated
Comment on lines 23 to 25
RUN apt-get install --no-install-recommends libpq-dev python-dev build-essential libsnappy-dev

RUN pip --no-cache-dir install poetry==1.6.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
RUN apt-get install --no-install-recommends libpq-dev python-dev build-essential libsnappy-dev
RUN pip --no-cache-dir install poetry==1.6.1
RUN pip --no-cache-dir install poetry==1.8.2

Not sure why I didn't notice this earlier, but this is an old version of poetry.

Try reverting the extra apt-get commands and seeing if this by itself fixes it.

@Shinnnyshinshin Shinnnyshinshin changed the title Update Poetry Lock file after python upgrade in Docker Upgrade Docker image to Python 3.10.13 after confirming Docker image build Apr 19, 2024
@Shinnnyshinshin Shinnnyshinshin changed the base branch from main to fix-310_13_container April 19, 2024 03:56
…ion-followup

* fix-310_13_container:
  Apply suggestions from code review
  pin python3-dev
  cleanup
  cleanup
  fix poetry install
  add back gcc pin
  combine
  no pseudo terminal
  only minimal deps
  test docker build in ci
  working
@Shinnnyshinshin Shinnnyshinshin changed the title Upgrade Docker image to Python 3.10.13 after confirming Docker image build Upgrade poetry.lock after updating Docker image to Python 3.10.13 Apr 19, 2024
Base automatically changed from fix-310_13_container to main April 19, 2024 08:18
poetry.lock Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

The command was run in a python 3.10.13 environment, which matches Docker.

Just to note, whether or not the lock command was run in a python 3.10.13 environment is largely irrelevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @Kilo59. This really helps clear up my understanding :)

@Shinnnyshinshin Shinnnyshinshin enabled auto-merge (squash) April 19, 2024 16:07
@Shinnnyshinshin Shinnnyshinshin merged commit 82270d4 into main Apr 19, 2024
15 checks passed
@Shinnnyshinshin Shinnnyshinshin deleted the m/z2-595/upgrade-python-version-followup branch April 19, 2024 16:09
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.

3 participants