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

Add Python3.11 to build matrix #317

Merged
merged 3 commits into from
Apr 11, 2023

Conversation

santi
Copy link
Collaborator

@santi santi commented Mar 9, 2023

Now that Python 3.11 is out, we should test the lib against it.

This PR adds Python 3.11 to the build matrix, but 3.10 is still used as the main version during releases++.
Also generated a requirements for 3.11 and added support in the Makefile.

Copy link
Collaborator

@tillahoffmann tillahoffmann left a comment

Choose a reason for hiding this comment

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

Great, thanks for the addition!

@tillahoffmann tillahoffmann merged commit 40a5e95 into testcontainers:master Apr 11, 2023
@tillahoffmann
Copy link
Collaborator

@santi, do you remember how long it took for the 3.11 dependencies to resolve? It's taking a rather long time (>1h) on my machine and am not sure whether I'm missing something.

@santi
Copy link
Collaborator Author

santi commented Apr 11, 2023

@tillahoffmann As far as I can remember it took a while to generate the requirements.txt, but nowhere near an hour. I'll take a look tomorrow and check it out myself.

@tillahoffmann
Copy link
Collaborator

Great, thank you!

@tillahoffmann
Copy link
Collaborator

tillahoffmann commented Apr 12, 2023

I've added a build to compile the requirements in #331. Here's an example of a slow build for 3.11 https://github.com/testcontainers/testcontainers-python/actions/runs/4681865266/jobs/8294990576. Any thoughts much appreciated!

@santi
Copy link
Collaborator Author

santi commented Apr 13, 2023

@tillahoffmann Looking into it. A fresh install on my M1 Macbook Air takes about ~20 seconds, but I'll try spinning up a new Python container and see how it behaves there. Probably a lot of non-standard config on my computer.

From the logs in the run you provided it seems to struggle to find compatible dependencies, which leads to backtracking and trying out most of the distributions available. This does not happen on my computer, so there might be some already built or resolved cache locally. A fresh Ubuntu image will hopefully trigger the problem, I'll try that out as a first theory.

@santi
Copy link
Collaborator Author

santi commented Apr 13, 2023

@tillahoffmann From a fresh install with ubuntu:20.04 running in a container, everything seems to work smoothly on the latest commit on master (a1aceac). Install takes about 30 seconds (running in Docker on a M1 Macbook Air), including downloads.

Only problem I had was installing pymyssql, which required a missing lib on standard Ubuntu distros.

A quick

apt update
apt install freetds-dev

fixed the problem and made everything install as expected, but this was probably not related to the backtracking error we saw in the Actions log.

Here is a full pastebin of the output from the run: https://pastebin.com/uADcPeSA

More recent runs also seem to complete in about 30 seconds, so unless the problem arises unexpectedly again, I guess this issue is resolved?

@tillahoffmann
Copy link
Collaborator

Thanks for the investigation, @santi! I think #333 resolved the issue. We had google-cloud-pubsub pinned to an ancient version which made the dependency resolution take a long time on 3.11.

@santi
Copy link
Collaborator Author

santi commented Apr 13, 2023

@tillahoffmann Happy to help!

A bit unrelated, but I just wanted to highlight this to you, as I see you are planning a major version release pretty soon: #314

I've implemented a change which makes the behaviour of the library more similar to the other implementations of testcontainers, by using Ryuk for cleaning up containers instead of having to rely on a fallback with a garbage collector with the del method (and also opens up more possibilites for using a container without having to keep a reference to the container at all times).

There has been a discussion going on in that PR on whether to enable the Ryuk cleanup by default, which would make the lib work in the same way as other implementations. HOWEVER that change in behaviour should be considered a breaking change, and requires a major version bump if we are to follow semantic versioning. I am open to making the last few changes required for enabling Ryuk by default on short notice in the PR. Is that something you could consider including in the v4 major release?

@tillahoffmann
Copy link
Collaborator

Yes, let's hold off with the major version release until #314 is addressed. Sorry I haven't been able to get to it yet. I'll be out of the office for the coming week but will have a look when I'm back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants