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 installation #1252

Merged
merged 2 commits into from
Jun 9, 2021
Merged

Conversation

sondrelg
Copy link
Contributor

@sondrelg sondrelg commented May 22, 2021

Type of changes

  • Bug fix

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

Version 1.1.6 of the install-poetry action is now compatible with the Python 3.10 pre-releases, so upgrading the version here should fix the issue referenced in #1216 🙂

Extra context

Unfortunately, while the original issue there is fixed, there seems to be another unrelated issue blocking the project from actually adding the 3.10 beta release into the matrix.

See this run, which outputs:

Installing dependencies from lock file

  SolverProblemError

  The current project's Python requirement (3.10.0b1) is not compatible with some of the required packages Python requirement:
    - dataclasses requires Python >=3.6, <3.7, so it will not be satisfied for Python 3.10.0b1
  
  Because no versions of black match >20.8b1,<21.0
   and black (20.8b1) depends on dataclasses (>=0.6), black (>=20.8b1,<21.0) requires dataclasses (>=0.6).
  Because dataclasses (0.8) requires Python >=3.6, <3.7
   and no versions of dataclasses match >=0.6,<0.8 || >0.8, dataclasses is forbidden.
  Thus, black is forbidden.
  So, because rich depends on black (^20.8b1), version solving failed.

I might be missing something obvious here, but this seems like an already resolved issue (see python-poetry/poetry#1413), and the fix seems to be using notation to tell Poetry that the dataclasses dependency is only relevant on certain Python versions (3.6), which the current pyproject.toml already seems to be doing.

If this really was a problem with the Python version I would expect version 3.7-3.9 to also fail this validation, so I wonder if Poetry has a bug tied to versioning of pre-releases somehow.

@sondrelg sondrelg force-pushed the python-3.10 branch 2 times, most recently from cc4e3db to ec31deb Compare May 22, 2021 21:56
@sondrelg sondrelg force-pushed the python-3.10 branch 8 times, most recently from 78f05a6 to c7d7c67 Compare May 24, 2021 08:53
@codecov
Copy link

codecov bot commented May 24, 2021

Codecov Report

Merging #1252 (0933eea) into master (c2f40e0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1252   +/-   ##
=======================================
  Coverage   99.82%   99.82%           
=======================================
  Files          69       69           
  Lines        6410     6410           
=======================================
  Hits         6399     6399           
  Misses         11       11           
Flag Coverage Δ
unittests 99.82% <ø> (ø)

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


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce4f18c...0933eea. Read the comment docs.

@sondrelg sondrelg force-pushed the python-3.10 branch 12 times, most recently from acad86c to 34e89d6 Compare May 24, 2021 12:06
@sondrelg sondrelg force-pushed the python-3.10 branch 3 times, most recently from 7745001 to 0933eea Compare May 24, 2021 12:24
@sondrelg sondrelg marked this pull request as ready for review May 24, 2021 12:30
with:
version: 1.1.4
virtualenvs-create: false
version: 1.1.6
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unintentionally possibly a little confusing. Just to clarify - the version parameter here specifies the Poetry version, while the @1.1.6 specifies the action version. They just happen to be the same.

Comment on lines +26 to +33
virtualenvs-in-project: true
- name: Install dependencies
run: poetry install
if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
- name: Format check with black
run: make format-check
run: |
source $VENV
make format-check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ran into some problems with the old setup, so switched from installing dependencies directly inside the runner environment, to installing dependencies to a venv and activating the venv in consequent steps.

Just for context, instead of source $VENV, we could have used source .venv/bin/activate, but since there's a windows combination in the matrix you would need to introduce conditional steps; on Windows you'd want ~source .venv/scripts/activate.

The $VENV environment variable is set inside the install-poetry action so upstream workflows don't need to worry about which OS you're on 🙂

@sondrelg sondrelg requested a review from willmcgugan May 24, 2021 12:38
@sondrelg sondrelg changed the title Add Python 3.10 to test matrix Upgrade Poetry installation May 24, 2021
@willmcgugan
Copy link
Collaborator

Thanks!

@willmcgugan willmcgugan merged commit b575697 into Textualize:master Jun 9, 2021
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.

None yet

2 participants