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

feat: Py310 & Runtime Python Verison Check #368

Closed
wants to merge 18 commits into from

Conversation

texastony
Copy link
Contributor

@texastony texastony commented Nov 3, 2021

Issue #, if available: No Issue

Description of changes:

This PR:

  • Fixes the failing static analysis checks by suppressing f-string lint requirements
  • Updates the Minor Python Versions used by CodeBuild to the latest available
  • Fixes the GitHub CI Workflows by updating the Python Setup used
  • Implements testing against Python 3.10.0 on CodeBuild
  • Implements a runtime check against the Python Runtime using the library.
  • Removes Python 3.5 testing and support.

The Runtime Check runs when __init__.py is loaded.
The Runtime Check:

  • Issues a WARNING if the Python Version will become unsupported.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

Issues a WARNING if the Python Version will become unsupported.
Prints an ERROR to STDERR if Python Version is unsupported.
Will start issuing WARNING for Python Version less than 3.6.
Will start printing ERROR for Python Version less than 3.5 AFTER Jan 1st, 2022.
@texastony texastony changed the title Py310 feat: Py310 & Runtime Python Verison Check Nov 3, 2021
@texastony texastony marked this pull request as ready for review November 3, 2021 18:08
@@ -36,6 +35,8 @@
StreamEncryptor,
)

check_python_version()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@farleyb-amazon I do not know how to force a Runtime Check, but I think this will work.
Do you know of another way?
I fear that this could run multiple times in an application if they load the library multiple times; but I guess I am willing to allow that to happen.
I also assume that __init__ will ALWAYS be parsed if any part of this library is used. I think that assumption is accurate... but I cannot swear it.

Copy link
Contributor

Choose a reason for hiding this comment

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

By my understanding __init__ will run anytime the module is imported, so this should be good. As you said it might run multiple times, but I don't mind that either.

@texastony texastony requested a review from josecorella November 3, 2021 18:15
.github/workflows/ci_tests.yaml Show resolved Hide resolved
@texastony texastony requested a review from josecorella November 3, 2021 19:24
This reverts: "ci(GitHub): test with Python 3.10"
Replaces check_python_version with _warn_deprecated_python.
_warn_deprecated_python is copied from the AWS SDK for Python (boto3/compat.py) and refactored for the ESDK.
No Error will ever be logged, only a warning for Pythons 2.7, 3.4, and 3.5.

Testing is added that not only checks the logic of the function, but also parses the SUPPORT_POLICY.rst for dates and ensures that the dates saved in compatability.py are accurate.
@@ -36,6 +35,8 @@
StreamEncryptor,
)

check_python_version()
Copy link
Contributor

Choose a reason for hiding this comment

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

By my understanding __init__ will run anytime the module is imported, so this should be good. As you said it might run multiple times, but I don't mind that either.

Comment on lines +28 to +35
"""Parses SUPPORT_POLICY.rst for Major Version Dates

The code here is strongly tied to the format of SUPPORT_POLICY.rst.
If the format changes, this logic will need to be refactored as well.

:rtype Mapping[str, str]
:return Mapping of keys from DEPRECATION_DATE_MAP to parsed date
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

This idea of parsing the policy file seems like a lot of complexity and future maintenance for not a lot of value. Is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the only way I can think of automatically ensuring the dates in the two files are the same.
I hate it. If we can just trust our selves to keep the two places in sync, then let's delete all this.
If we cannot, then this is the best I can think of.

In terms of Data: The Boto3 library does not bother checking that the dates are aligned. That team just trusts that the dates entered are accurate.

I would prefer trashing all this test logic, and having faith that the date is right.

@texastony
Copy link
Contributor Author

Replaced by #370, #371, #372

@texastony texastony closed this Nov 8, 2021
@texastony texastony deleted the py310 branch August 29, 2022 18:35
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.

3 participants