-
-
Notifications
You must be signed in to change notification settings - Fork 639
feat(toolchain): drop 3.8 and print info level messages about it #3387
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 all commits
64f91a7
9b4a71a
f14bea6
37a9b85
27de730
f07c7db
c07bfea
1d8efe1
2fabab1
c0d1d65
a8ed451
4e2cac0
a36758a
8be6135
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,9 +114,29 @@ def _pip_parse(self, module_ctx, pip_attr): | |
| version = python_version, | ||
| )) | ||
|
|
||
| self._platforms[python_version] = _platforms( | ||
| python_version = python_version, | ||
| full_python_version = full_version( | ||
| version = python_version, | ||
| minor_mapping = self._minor_mapping, | ||
| fail_on_err = False, | ||
| ) | ||
| if not full_python_version: | ||
| # NOTE @aignas 2025-11-18: If the python version is not present in our | ||
| # minor_mapping, then we will not register any packages and then the | ||
| # select in the hub repository will fail, which will prompt the user to | ||
| # configure the toolchain correctly and move forward. | ||
| self._logger.info(lambda: ( | ||
|
Comment on lines
+123
to
+127
Collaborator
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. Why log with a lambda?
Collaborator
Author
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. Me and @rickeylev decided to have
Collaborator
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. Interesting, does Bazel not have the "deferred formatting" that core python logging has? logger.info("value is: %s", value)The string formatting will not happen unless the log level is INFO or lower.
Collaborator
Author
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. Hmmm, I can't remember if I had something like that prototyped earlier or not, maybe we should look into this. should not be too difficult and would definitely remove the need for doing those inline lambdas. That said, the current way allows us to use the |
||
| "Ignoring pip python version '{version}' for hub " + | ||
| "'{hub}' in module '{module}' because there is no registered " + | ||
| "toolchain for it." | ||
| ).format( | ||
|
Comment on lines
+127
to
+131
Collaborator
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. Probably more readable as a
Collaborator
Author
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. I'll leave this as is so that it is not outside the lambda. |
||
| hub = self.name, | ||
| module = self.module_name, | ||
| version = python_version, | ||
| )) | ||
| return | ||
|
|
||
| self._platforms[python_version] = _platforms( | ||
| python_version = full_python_version, | ||
| config = self._config, | ||
| ) | ||
| _set_get_index_urls(self, pip_attr) | ||
|
|
@@ -280,13 +300,10 @@ def _detect_interpreter(self, pip_attr): | |
| path = pip_attr.python_interpreter, | ||
| ) | ||
|
|
||
| def _platforms(*, python_version, minor_mapping, config): | ||
| def _platforms(*, python_version, config): | ||
| platforms = {} | ||
| python_version = version.parse( | ||
| full_version( | ||
| version = python_version, | ||
| minor_mapping = minor_mapping, | ||
| ), | ||
| python_version, | ||
| strict = True, | ||
| ) | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.