-
-
Notifications
You must be signed in to change notification settings - Fork 639
refactor: read migrated native flags through a centralized accessor function #3290
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
refactor: read migrated native flags through a centralized accessor function #3290
Conversation
Summary of ChangesHello @gregestren, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural change to how Python-related build flags are handled within the Bazel rules. It establishes a centralized accessor function that intelligently determines whether to retrieve flag values from native Bazel fragments or Starlark build settings. This change is essential for future compatibility with Bazel 9.0 and beyond, which will allow disabling native fragments, and provides a clear path for the Starlarkification of existing flags while offering a developer-friendly override for testing. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a centralized accessor function, read_possibly_native_flag, to handle reading Python-related flags. This change is a great step towards supporting both native and Starlark-defined flags, which will improve compatibility across different Bazel versions. The implementation correctly abstracts away the logic for choosing between native fragments and Starlark build settings. I have one suggestion to improve the maintainability of the new flag definition map.
5f77ecd to
4288fd6
Compare
…#2301) Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.2.2 to 2.2.3. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/urllib3/urllib3/releases">urllib3's releases</a>.</em></p> <blockquote> <h2>2.2.3</h2> <h2>🚀 urllib3 is fundraising for HTTP/2 support</h2> <p><a href="https://sethmlarson.dev/urllib3-is-fundraising-for-http2-support">urllib3 is raising ~$40,000 USD</a> to release HTTP/2 support and ensure long-term sustainable maintenance of the project after a sharp decline in financial support for 2023. If your company or organization uses Python and would benefit from HTTP/2 support in Requests, pip, cloud SDKs, and thousands of other projects <a href="https://opencollective.com/urllib3">please consider contributing financially</a> to ensure HTTP/2 support is developed sustainably and maintained for the long-haul.</p> <p>Thank you for your support.</p> <h2>Features</h2> <ul> <li>Added support for Python 3.13. (<a href="https://github.com/urllib3/urllib3/issues/3473">#3473</a>)</li> </ul> <h2>Bugfixes</h2> <ul> <li>Fixed the default encoding of chunked request bodies to be UTF-8 instead of ISO-8859-1. All other methods of supplying a request body already use UTF-8 starting in urllib3 v2.0. (<a href="https://github.com/urllib3/urllib3/issues/3053">#3053</a>)</li> <li>Fixed ResourceWarning on CONNECT with Python < 3.11.4 by backporting <a href="https://github.com/python/cpython/issues/103472">python/cpython#103472</a>. (`<a href="https://github.com/urllib3/urllib3/issues/3252">#3252</a>)</li> <li>Adjust tolerance for floating-point comparison on Windows to avoid flakiness in CI (<a href="https://github.com/urllib3/urllib3/issues/3413">#3413</a>)</li> <li>Fixed a crash where certain standard library hash functions were absent in restricted environments. (<a href="https://github.com/urllib3/urllib3/issues/3432">#3432</a>)</li> <li>Fixed mypy error when adding to <code>HTTPConnection.default_socket_options</code>. (<a href="https://github.com/urllib3/urllib3/issues/3448">#3448</a>)</li> </ul> <h2>HTTP/2 (experimental)</h2> <p>HTTP/2 support is still in early development.</p> <ul> <li>Excluded Transfer-Encoding: chunked from HTTP/2 request body (<a href="https://github.com/urllib3/urllib3/issues/3425">#3425</a>)</li> <li>Added version checking for <code>h2</code> (<a href="https://pypi.org/project/h2/">https://pypi.org/project/h2/</a>) usage. Now only accepting supported h2 major version 4.x.x. (<a href="https://github.com/urllib3/urllib3/issues/3290">#3290</a>)</li> <li>Added a probing mechanism for determining whether a given target origin supports HTTP/2 via ALPN. (<a href="https://github.com/urllib3/urllib3/issues/3301">#3301</a>)</li> <li>Add support for sending a request body with HTTP/2 (<a href="https://github.com/urllib3/urllib3/issues/3302">#3302</a>)</li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/urllib3/urllib3/compare/2.2.2...2.2.3">https://github.com/urllib3/urllib3/compare/2.2.2...2.2.3</a></p> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/urllib3/urllib3/blob/main/CHANGES.rst">urllib3's changelog</a>.</em></p> <blockquote> <h1>2.2.3 (2024-09-12)</h1> <h2>Features</h2> <ul> <li>Added support for Python 3.13. (<code>[#3473](urllib3/urllib3#3473) <https://github.com/urllib3/urllib3/issues/3473></code>__)</li> </ul> <h2>Bugfixes</h2> <ul> <li>Fixed the default encoding of chunked request bodies to be UTF-8 instead of ISO-8859-1. All other methods of supplying a request body already use UTF-8 starting in urllib3 v2.0. (<code>[bazel-contrib#3053](urllib3/urllib3#3053) <https://github.com/urllib3/urllib3/issues/3053></code>__)</li> <li>Fixed ResourceWarning on CONNECT with Python <!-- raw HTML omitted -->`__)</li> <li>Adjust tolerance for floating-point comparison on Windows to avoid flakiness in CI (<code>[#3413](urllib3/urllib3#3413) <https://github.com/urllib3/urllib3/issues/3413></code>__)</li> <li>Fixed a crash where certain standard library hash functions were absent in restricted environments. (<code>[#3432](urllib3/urllib3#3432) <https://github.com/urllib3/urllib3/issues/3432></code>__)</li> <li>Fixed mypy error when adding to <code>HTTPConnection.default_socket_options</code>. (<code>[#3448](urllib3/urllib3#3448) <https://github.com/urllib3/urllib3/issues/3448></code>__)</li> </ul> <h2>HTTP/2 (experimental)</h2> <p>HTTP/2 support is still in early development.</p> <ul> <li> <p>Excluded Transfer-Encoding: chunked from HTTP/2 request body (<code>[#3425](urllib3/urllib3#3425) <https://github.com/urllib3/urllib3/issues/3425></code>__)</p> </li> <li> <p>Added version checking for <code>h2</code> (<a href="https://pypi.org/project/h2/">https://pypi.org/project/h2/</a>) usage.</p> <p>Now only accepting supported h2 major version 4.x.x. (<code>[bazel-contrib#3290](urllib3/urllib3#3290) <https://github.com/urllib3/urllib3/issues/3290></code>__)</p> </li> <li> <p>Added a probing mechanism for determining whether a given target origin supports HTTP/2 via ALPN. (<code>[bazel-contrib#3301](urllib3/urllib3#3301) <https://github.com/urllib3/urllib3/issues/3301></code>__)</p> </li> <li> <p>Add support for sending a request body with HTTP/2 (<code>[bazel-contrib#3302](urllib3/urllib3#3302) <https://github.com/urllib3/urllib3/issues/3302></code>__)</p> </li> </ul> <h2>Deprecations and Removals</h2> <ul> <li>Note for downstream distributors: the <code>_version.py</code> file has been removed and is now created at build time by hatch-vcs. (<code>[#3412](urllib3/urllib3#3412) <https://github.com/urllib3/urllib3/issues/3412></code>__)</li> <li>Drop support for end-of-life PyPy3.8 and PyPy3.9. (<code>[#3475](urllib3/urllib3#3475) <https://github.com/urllib3/urllib3/issues/3475></code>__)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/urllib3/urllib3/commit/2458bfcd3dacdf6c196e98d077fc6bb02a5fc1df"><code>2458bfc</code></a> Release 2.2.3</li> <li><a href="https://github.com/urllib3/urllib3/commit/9b25db6d00e43858d49303ae55c43bc4a9832668"><code>9b25db6</code></a> Only attempt to publish for upstream</li> <li><a href="https://github.com/urllib3/urllib3/commit/b9adeef8501180cd7d04cc3fb90bed4bbc34b1bb"><code>b9adeef</code></a> Drop support for EOL PyPy3.8 and PyPy3.9</li> <li><a href="https://github.com/urllib3/urllib3/commit/b1d4649d43375f11a3072b4d9b5d33425d123bae"><code>b1d4649</code></a> Add explicit support for Python 3.13</li> <li><a href="https://github.com/urllib3/urllib3/commit/cc42860721836febf3fb6ebb485ed27d7f80122d"><code>cc42860</code></a> Bump cryptography from 42.0.4 to 43.0.1 (<a href="https://github.com/urllib3/urllib3/issues/3470">#3470</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/3dae2e9b30d2e39bf20daea2353aa7ef055640cf"><code>3dae2e9</code></a> Bump pypa/gh-action-pypi-publish from 1.9.0 to 1.10.1 (<a href="https://github.com/urllib3/urllib3/issues/3469">#3469</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/1e94feb2a671bf28721114dfea1105a2c1f91788"><code>1e94feb</code></a> Revert "Add TLS settings for HTTP/2 (<a href="https://github.com/urllib3/urllib3/issues/3456">#3456</a>)" (<a href="https://github.com/urllib3/urllib3/issues/3466">#3466</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/aa73abc7b22a4a67e0ee957f5a3031109f73d3d9"><code>aa73abc</code></a> Bump actions/setup-python from 5.1.0 to 5.2.0 (<a href="https://github.com/urllib3/urllib3/issues/3468">#3468</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/abbfbcb1dd274fc54b4f0a7785fd04d59b634195"><code>abbfbcb</code></a> Add 1.26.20 to changelog and make the publish workflow the same (<a href="https://github.com/urllib3/urllib3/issues/3464">#3464</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/d48061505e72271116c5a33b04dbca6273f2a737"><code>d480615</code></a> Add TLS settings for HTTP/2 (<a href="https://github.com/urllib3/urllib3/issues/3456">#3456</a>)</li> <li>Additional commits viewable in <a href="https://github.com/urllib3/urllib3/compare/2.2.2...2.2.3">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This lets Python logic support either Starlark- or native-defined flags, based on Bazel support:
ctx.fragments.pyandctx.fragments.bazel_pyis setAlso check in a Starlark version of
--experimental_python_import_all_repositoriesfor testing.This is a no-op. New flag sources of truth don't take effect without a supporting bazel version that sets appropriate incompatible flags.
Caveats:
configuration_fieldas implemented. We can loosen the incompatible flag lockdown if that's an issue.For #3252
TODO list