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 pyyaml to 6.0.1 #445

Merged
merged 7 commits into from
Aug 17, 2023
Merged

Conversation

juagargi
Copy link
Member

@juagargi juagargi commented Jul 25, 2023

Previous versions are incompatible with Cython 3.0.0, which was released just the previous week.
Upgrading to >5.4.1 seems too complex, given that 6.0.1 looks compatible with our code.

See:
yaml/pyyaml#724
yaml/pyyaml#726


This change is Reviewable

Previous versions are incompatible with Cython 3.0.0, which was released
just the previous week.
See yaml/pyyaml#724
sqlparse to 0.4.4
requests to 2.31.0
django to 3.2.20
cryptography to 41.0.2
@juagargi juagargi force-pushed the juagargi/pyyaml_cython_incompatibility branch 21 times, most recently from 0c99522 to 4501aa4 Compare July 25, 2023 14:07
@juagargi juagargi mentioned this pull request Jul 26, 2023
1 task
@juagargi juagargi force-pushed the juagargi/pyyaml_cython_incompatibility branch 5 times, most recently from f0999fb to 9b947b3 Compare August 2, 2023 10:26
@juagargi juagargi force-pushed the juagargi/pyyaml_cython_incompatibility branch from 9b947b3 to 1dc6536 Compare August 4, 2023 07:46
@juagargi juagargi force-pushed the juagargi/pyyaml_cython_incompatibility branch from 1dc6536 to 704b5a4 Compare August 4, 2023 09:54
The new machine runs a new version of Docker. We require changes.
The new machine runs python 3.10, we require changes.

deleteme
Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions / 0 of 1 LGTMs obtained / 0 of 1 approvals obtained


.dockerignore line 3 at r1 (raw file):

.circleci

We don't need the files in .circleci to be included in the production docker images. See comment on line 222 in file .circleci/config.yml


README.md line 50 at r1 (raw file):

#         apt install libssl1.0
# - The `psycopg2-binary` package may fail if libpq-dev is not installed.
#         apt install libpq-dev

Isn't also libffi-dev required if building wheel cffi from source?


.circleci/config.yml line 63 at r1 (raw file):

            python3 -m venv /tmp/venv 2>&1
            . /tmp/venv/bin/activate
            pip3 install -U pip  # need pip >= 19.0 to install cryptography

This should not be required, in the venv pip and pip3 are identical.

Suggestion:

pip

Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions / 0 of 1 LGTMs obtained / 0 of 1 approvals obtained

Copy link
Contributor

@FR4NK-W FR4NK-W left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 1 LGTMs obtained

@juagargi juagargi merged commit efa5a14 into develop Aug 17, 2023
4 of 5 checks passed
@juagargi juagargi deleted the juagargi/pyyaml_cython_incompatibility branch August 17, 2023 08:40
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