-
Notifications
You must be signed in to change notification settings - Fork 95
feat: provide and use Python version support check #832
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
Changes from 1 commit
6316d70
a136ad7
45cd647
25225fa
1d567c0
f1dbbb4
e3fd56f
8b1dfb1
4b9208e
5cf8652
7d8b1c7
cda8e27
db92fac
118a7c8
f1c46aa
474ec0d
2083b49
4ea124e
c5949c4
6fc1473
8829e20
bdb6260
7896664
54a5611
2e2990b
35a2074
cd64832
e36414a
b6245ca
3a897ba
814ea63
8dee04b
95f4777
a34ec15
a04e2e6
8b3f337
29896cf
d421f98
ac76f7a
e093a96
187f848
b575a16
ecb7211
3c587cf
fbe0f0b
28b0b32
21c9aec
e24c13d
dbc3a26
d472bae
0550f83
491b695
6338389
05bcf6f
fc224b7
a6c40ce
d0414b2
835df53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,10 +37,10 @@ class VersionInfo(NamedTuple): | |
| python_beta: Optional[datetime.date] | ||
| python_start: datetime.date | ||
| python_eol: datetime.date | ||
| gapic_start: Optional[datetime.date] = None | ||
| gapic_start: Optional[datetime.date] = None # unused | ||
| gapic_deprecation: Optional[datetime.date] = None | ||
| gapic_end: Optional[datetime.date] = None | ||
| dep_unpatchable_cve: Optional[datetime.date] = None | ||
| dep_unpatchable_cve: Optional[datetime.date] = None # unused | ||
|
|
||
|
|
||
| PYTHON_VERSION_INFO: Dict[Tuple[int, int], VersionInfo] = { | ||
|
|
@@ -97,7 +97,8 @@ def _flatten_message(text: str) -> str: | |
| return textwrap.dedent(text).strip().replace("\n", " ") | ||
|
|
||
|
|
||
| def check_python_version(today: Optional[datetime.date] = None) -> PythonVersionStatus: | ||
| def check_python_version(package: Optional[str] = "this package", | ||
| today: Optional[datetime.date] = None) -> PythonVersionStatus: | ||
| """Check the running Python version and issue a support warning if needed. | ||
|
|
||
| Args: | ||
|
|
@@ -146,9 +147,9 @@ def min_python(date: datetime.date) -> str: | |
| message = _flatten_message( | ||
| f""" | ||
| You are using a non-supported Python version ({py_version_str}). | ||
| You will receive no updates to this client library. We suggest | ||
| Google will not post any further updates to {package}. We suggest | ||
| you upgrade to the latest Python version, or at least Python | ||
| {min_python(today)}, and then update this library. | ||
| {min_python(today)}, and then update {package}. | ||
| """ | ||
| ) | ||
| logging.warning(message) | ||
|
|
@@ -159,11 +160,10 @@ def min_python(date: datetime.date) -> str: | |
| message = _flatten_message( | ||
| f""" | ||
| You are using a Python version ({py_version_str}) past its end | ||
| of life. This client library will continue receiving critical | ||
| bug fixes on a best-effort basis, but not any other fixes or | ||
| of life. Google will update {package} with critical | ||
| bug fixes on a best-effort basis, but not with any other fixes or | ||
| features. We suggest you upgrade to the latest Python version, | ||
| or at least Python {min_python(today)}, and then update this | ||
| library. | ||
| or at least Python {min_python(today)}, and then update {package}. | ||
| """ | ||
| ) | ||
| logging.warning(message) | ||
|
|
@@ -172,12 +172,12 @@ def min_python(date: datetime.date) -> str: | |
| if gapic_deprecation <= today <= gapic_end: | ||
| message = _flatten_message( | ||
| f""" | ||
| You are using a Python version ({py_version_str}), which new | ||
| releases of this client library will stop supporting when it | ||
| You are using a Python version ({py_version_str}), | ||
| which Google will stop supporting in {package} when it | ||
| reaches its end of life ({version_info.python_eol}). We | ||
| suggest you upgrade to the latest Python version, or at least | ||
| Python {min_python(version_info.python_eol)}, and then update | ||
| this library. | ||
| suggest you upgrade to the latest Python version, or at | ||
| least Python {min_python(version_info.python_eol)}, and | ||
| then update {package}. | ||
| """ | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think these message (or at least templates) would be better as constants, instead of embedded in the function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, for the same reason as the other case. They are defined and used directly only once, and the substitution strings and the text are closely related to the logic of the function. I think it reads more clearly to have them defined inline, as they are now. |
||
| logging.warning(message) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary? You could use parentheses to build the multi-line strings, without having to post-process it at runtime:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contrariwise, is the optimization you suggest really necessary?
The advantage of the way I have it is that it makes changing the message simpler for developers: just write and re-wrap the text. The cost is that, yes, it does get called at run time, but only upon initialization and only once per message that is printed. So yes, it could add up, but it doesn't seem like that would be substantial.
If you feel strongly about it I'll change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I raised this more for simplicity rather than performance concerns. Python has built-in functionality for building long strings, so adding a custom helper for this seemed unnecessary to me. But this is more of a nit; I don't think this helper is too complex, and I don't feel too strongly about it
If we keep it, can we add some unit tests or something around it though?