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

Maintenance: Change the project.toml to restrict Python 3.10 version #1404

Closed
2 tasks done
leandrodamascena opened this issue Aug 2, 2022 · 14 comments
Closed
2 tasks done
Labels
internal Maintenance changes wontfix This will not be worked on

Comments

@leandrodamascena
Copy link
Contributor

Summary

The latest version of the Lambda runtime for Python is 3.9 and this project must have the same restriction.
The way project.toml is configured, the package on PyPi adds Python 3.10 as a supported version.

image

Why is this needed?

Keep the same python versions supported by Lambda Runtime

Which area does this relate to?

Governance, Other

Solution

No response

Acknowledgment

@leandrodamascena leandrodamascena added internal Maintenance changes triage Pending triage from maintainers labels Aug 2, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Aug 2, 2022

Thanks for opening your first issue here! We'll come back to you as soon as we can.

@jplock
Copy link

jplock commented Aug 3, 2022

Locally I’m running Python 3.10 and I install the powertools via pip. When I deploy through SAM I’m using the layer. I don’t think we should limit being able to install this package when running 3.10.

@leandrodamascena
Copy link
Contributor Author

leandrodamascena commented Aug 3, 2022

Hi @jplock, this is really an excellent point for us to debate here. It really works to use Python 3.10 to install via pip or even use the source code of this project, I've done some tests here and it's working "fine". But I think we should pay attention to a few points:

1 - Ok we have Python 3.10 installed and the project working, but we must remember that the highest version allowed by the Lambda Runtime is 3.9. So using local Python 3.10 (even via SAM) to run on the Lambda 3.9 runtime can cause some issues.

2 - Many good things were added in Python 3.10 and of course they are not supported by Python 3.9. Like PEP-0634 and PEP-0604

3 - Now imagine a scenario where a contributor wants to submit a PR for the project, but the contributor has Python 3.10 installed in the local environment and writes code supported only by Python 3.10. When the contributor submits the PR, all pipelines will fail because this project uses matrix to build a version and Python 3.10 isn't supported.
https://github.com/awslabs/aws-lambda-powertools-python/blob/bb1eda742583d58790219942a49a5d2489ea91ea/.github/workflows/python_build.yml#L29

4 - Some examples that work in Python 3.10 and doesn't work in Python 3.9
image
image

So based on these facts I think it's protection/better governance to add this restriction to the Python version.

I'd like to hear your point of view to get things I'm missing.

@jplock
Copy link

jplock commented Aug 3, 2022

I understand the points but who is our target audience: people who will be submitting changes to the library or people who want to use the library? I think people using the library will be more common and we shouldn’t limit how it can be installed if they happen to use the latest available Python release on their local machines. Maybe we could check with the Lambda runtime team on when 3.10 support is coming? If it’s soon, maybe we skip this.

@leandrodamascena
Copy link
Contributor Author

I agree with you that people using the library is the most common situation. But in my opinion even those people shouldn't be affected because the purpose of this project is to help projects using AWS Lambda and those projects must use a Python version below 3.10 because Lambda Runtime doesn't support 3.10+.
But I actually don't have enough data and I don't think I'm the right person to define this as I'm not part of the AWS team and I don't have access to all the information. I think we need to hear the views of the maintainers. It's just my opinion and I could be wrong.

Yessss!! Talking to the Lambda runtime team can be awesome as they certainly have a roadmap and can provide more information.

Btw, there is an issue aws/aws-lambda-base-images#31 where people are asking for this feature/support.

@heitorlessa
Copy link
Contributor

We'll look into this for next week's release, as we had some funky expressions with Poetry before. I appreciate everyone's point of view. For correctness, and specially given the number of breaking changes in 3.10+, we should update our support to 3.9 only.

Lambda will take some time to provide support for 3.10. When they do, it'll take us some time to address some difficult typing breakage too.

@peterschutt
Copy link
Contributor

peterschutt commented Aug 6, 2022

What about those of us running 3.10 in containers via lambda?

Due to the container runtime I could be running 3.11.0b5 as we speak, not that I'd expect this library to support it. But 3.10 has been GA for ~10 months and it is reasonable to expect I wouldn't be the only one running it in a container and depending on this library.

@heitorlessa
Copy link
Contributor

I didn't see that coming. Sometimes I forget about OCI because our tenets track managed runtimes, and that's a blind spot.

This argument is valid and we shouldn't do it since we don't know how many would be impacted; classic Hyrum's Law.

Here's where I'd like help from everyone - how do we best communicate that we don't make guarantees 3.10+ will work?

This applies for any future Python version not yet supported as a Lambda managed runtime.

@peterschutt
Copy link
Contributor

classic Hyrum's Law

one of the best

If I didn't come across this issue today, I wouldn't have known 3.10 was unsupported.

I've looked at awslambdaric as an example, they have pretty much the same classifiers as you, and the same python version specified, but because they use setuptools their classifiers are packaged verbatim:

image

Whereas poetry seems to assume that the 3.10 classifier is an omission if the python dependency specification includes 3.10. Skimming over the docs, I couldn't see a way to override it either and I'm not sure how I feel about that. Still though, even if the classifier could be rectified, the Requires: Python ... still includes 3.10 in either case:

lambdaric:

image

you:

image

I think awslambdaric is a good example though, they pretty prominently state they only support up to 3.9 in their readme, but they don't go so far as to actually restrict 3.10+ and a github search including "awslambdaric" and "3.10" brings up plenty of dockerfiles combining the two.

Similarly, I think it would be a shame for this library to actually put an upper limit on the python version.

Some suggestions:

  1. State the version support policy prominently - those tenets are right at the bottom (I'd never actually made it that far down the homepage 😬).
  2. Strengthen the wording of the first tenet. IMO it would have been a pretty narrow interpretation of the words "optimise for" to go ahead and exclude python 3.10 from support altogether.
  3. You could include a compatibility section of the docs that summarises known version incompatibilities and links to tracking issues.
  4. Perhaps a runtime warning on first import if not a supported version - maybe a bit extreme though.
  5. If you decide you do want to cap the upper limit in line with the supported runtimes, you could accept that the 3.10 ship has sailed and make the python dependency >=3.6,<=3.10 - that way you wouldn't break anyone now, and wouldn't be forced into 3.11 support when it becomes GA (or whenever poetry decides to add the classifier for you).

You mentioned earlier:

... and specially given the number of breaking changes in 3.10+ ...

Are these breaks tracked anywhere? Searching the issues for "3.10" doesn't come up with much and I've been humming along on 3.10 without any problems until I came across this issue today. Most likely I'm using areas of the lib that aren't affected so it would be good to get a better understanding if you can point me at anything to read.

Cheers:)

@heitorlessa
Copy link
Contributor

heitorlessa commented Aug 7, 2022 via email

@leandrodamascena
Copy link
Contributor Author

What about those of us running 3.10 in containers via lambda?

Due to the container runtime I could be running 3.11.0b5 as we speak, not that I'd expect this library to support it. But 3.10 has been GA for ~10 months and it is reasonable to expect I wouldn't be the only one running it in a container and depending on this library.

Ohhh 😮.. I think that's a great argument for not restricting the Python version and I didn't think about this point either. Right now this project could be running on thousands of containers using Python 3.10+ and this could have a big impact.

👏

@heitorlessa
Copy link
Contributor

I couldn't carve bandwidth to look into suggestions by @peterschutt but I've just closed the PR to signal our intent to not go ahead with the change given the customer impact.

I'm inclined to close on the basis that any customer using OCI could use a newer Python version that might work, and sometimes it could be a minor change to allow customers benefit from faster versions. Our first tenet covers the intent well regardless.

The only minor change I'd think we should make is to update our top note pointing customers to our Tenets (🤦 noticed we haven't updated to add .NET yet!)

image

@heitorlessa heitorlessa added the wontfix This will not be worked on label Sep 1, 2022
@peterschutt
Copy link
Contributor

Many thanks from those of us living on the edge:) One month till 3.11!

@heitorlessa heitorlessa closed this as not planned Won't fix, can't repro, duplicate, stale Sep 30, 2022
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Maintenance changes wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants