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

update lockfile to bump pillow #11938

Merged
merged 4 commits into from
Jan 19, 2023
Merged

update lockfile to bump pillow #11938

merged 4 commits into from
Jan 19, 2023

Conversation

m-vdb
Copy link
Collaborator

@m-vdb m-vdb commented Jan 18, 2023

Proposed changes:

  • Update lockfile to bump Pillow
  • Update wheel in docker containers

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@@ -1878,14 +1883,14 @@ packaging = ">=20.3,<21.0"

[[package]]
name = "pillow"
version = "9.3.0"
version = "9.4.0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pillow version bump

@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 18, 2023

@rasa-jmac when bumping the dependencies, I noticed that wheel isn't a dependency or rasa, and he's not in the lock file. Now, wheel is a packaging dependency, so IMO it's only relevant at the build step; I'm wondering if it's not installed with the Python version that is installed in the docker containers. Any ideas? cc @ancalita

@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 18, 2023

FYI @sanchariGr

@m-vdb m-vdb enabled auto-merge January 18, 2023 12:27
@m-vdb m-vdb added the tools:clear-poetry-cache-unit-tests Clear poetry cache for the unit tests label Jan 18, 2023
@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 18, 2023

builds are currently failing because of this issue. Trying to pin pydantic to 1.10.2, generating the lockfile and pushing to this branch when it's done. The last time I ran it it took me ~1h to generate the lockfile....

@m-vdb m-vdb removed the tools:clear-poetry-cache-unit-tests Clear poetry cache for the unit tests label Jan 18, 2023
@ancalita
Copy link
Member

wheel seems to be a dependency of other direct dependencies of rasa (such as setuptools). Perhaps if setuptools gets upgraded to latest version 66.0.0, wheel gets resolved to the version we need for the update?
Strangely, when I checked the poetry.lock in this PR, it had setuptools removed, although it is specified in pyproject.toml.

@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 18, 2023

but @ancalita if this was the case, wheel would be in the lock file. It isn't 🤔

@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 18, 2023

Strangely, when I checked the poetry.lock in this PR, it had setuptools removed, although it is specified in pyproject.toml.

interestingly @ancalita , this was an issue with poetry < 1.2 (see this comment). It's probably the same thing with wheel...

@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 18, 2023

@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 18, 2023

@ancalita I'm not sure where to pin wheel exactly, as it's not a dependency of rasa. I think in the Dockerfile could be a good place:

pip install --no-cache-dir -U "pip==22.*"

thoughts?

Edit: I went ahead and implemented it in 0bd52e6 to save some time in case it's the valid approach; let me know if it's not the right place

@ancalita
Copy link
Member

@ancalita I'm not sure where to pin wheel exactly, as it's not a dependency of rasa. I think in the Dockerfile could be a good place:

pip install --no-cache-dir -U "pip==22.*"

thoughts?

Edit: I went ahead and implemented it in 0bd52e6 to save some time in case it's the valid approach; let me know if it's not the right place

I can't think of a different way to update wheel either, unless we add it to pyproject.toml which I don't think it's a good idea. This Dockerfile needs updating too; if the other ones use this as a base, might not be necessary to amend the Dockerfiles you've amended?

@m-vdb
Copy link
Collaborator Author

m-vdb commented Jan 19, 2023

@ancalita I've updated it for consistency, but after a quick look at our CI pipeline, I am not sure that this Dockerfile at the root of the repo is used for building the images that we released.

@m-vdb m-vdb removed the request for review from rasa-jmac January 19, 2023 10:45
@m-vdb m-vdb disabled auto-merge January 19, 2023 13:51
@m-vdb m-vdb merged commit 3c8d32d into 3.4.x Jan 19, 2023
@m-vdb m-vdb deleted the ATO-658-bump-pillow branch January 19, 2023 13:51
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