-
Notifications
You must be signed in to change notification settings - Fork 346
Fixed issues on passed scopes as set #1145
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
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
The documentation seems to indicate the scope is a sequence, so it could either be a list or set. I think the general case has been a list. Can you create an issue documenting what issue this is causing for you? Also, please sign the CLA :) |
This change ensures that if a set of scopes is passed to Credentials, it is converted to a list. This prevents issues with JSON serialization (to_json failure) and ensures consistent mutability behavior for the scopes property. Fixes #1145
|
Closing in deference to a PR #1898 that addresses some minor issues with this approach. |
This change ensures that if a set of scopes is passed to Credentials, it is converted to a list. This prevents issues with JSON serialization (to_json failure) and ensures consistent mutability behavior for the scopes property. Also updated tests/test__oauth2client.py to handle the type change (set -> list) in equality checks. Fixes #1145
This change ensures that if a set of scopes is passed to Credentials, it is converted to a list. This prevents issues with JSON serialization (to_json failure) and ensures consistent mutability behavior for the scopes property. Also updated tests/test__oauth2client.py to handle the type change (set -> list) in equality checks. Fixes #1145
This change ensures that if a set of scopes is passed to Credentials, it is converted to a list. This prevents issues with JSON serialization (to_json failure) and ensures consistent mutability behavior for the scopes property. Also updated tests/test__oauth2client.py to handle the type change (set -> list) in equality checks. Fixes #1145
This change ensures that if a set of scopes is passed to Credentials, it is converted to a list. This prevents issues with JSON serialization (to_json failure) and ensures consistent mutability behavior for the scopes property. Also updated tests/test__oauth2client.py to handle the type change (set -> list) in equality checks. Fixes #1145
This change ensures that if a set of scopes is passed to Credentials, it is converted to a list. This prevents issues with JSON serialization (to_json failure) and ensures consistent mutability behavior for the scopes property. Also updated tests/test__oauth2client.py to handle the type change (set -> list) in equality checks. Fixes #1145
This PR addresses an issue where passing a `set` of scopes to `Credentials` causes `to_json` to fail and introduces inconsistent mutability behavior if handled via a property accessor. Changes: - Modified `google.oauth2.credentials.Credentials.__init__` to convert `scopes` to a `list` if it is a `set`. - Updated `google.oauth2.credentials.Credentials.scopes` property docstring to `Optional[Sequence[str]]`. - Added a regression test `test_init_with_set_scopes` in `tests/oauth2/test_credentials.py` to verify the fix, ensuring type conversion, object stability, and serialization support. - Updated `tests/test__oauth2client.py` to relax strict equality checks on `scopes` (comparing as `sets`) to accommodate the new behavior where internal storage is enforced as a `list`. This PR supercedes PR #1145 which potentially introduced a mutability inconsistency and failed to address a type correctness issue. This PR avoids both of those concerns. 1. Mutability Inconsistency: If `._scopes` is a `set`, the `scopes` property returns a new `list` every time it is accessed. * If the user modifies this list (e.g. `creds.scopes.append("new_scope")`), the modification is lost because the next access returns a fresh list **from the underlying set**. * If `_scopes` was originally a list (passed in `__init__`), `creds.scopes` returns the same list reference, so modifications are preserved. This inconsistent behavior between "set-initialized" and "list-initialized" credentials is a potential bug trap. 2. Type correctness: The scopes argument in `__init__` is documented as `Sequence[str]`. A set is not a `Sequence` (it's not indexable/ordered). While Python is lenient, it is better to normalize the input at the boundary (`__init__`) rather than in the accessor. Moving the conversion logic to `__init__` ensures: * _scopes is always a list (or `None`). * Access to .scopes always returns the stored list (consistent mutability). * `to_json` works correctly because it accesses `self.scopes` (or self._scopes). --- *PR created automatically by Jules for task [15605314498929918698](https://jules.google.com/task/15605314498929918698) started by @chalmerlowe* --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Chalmer Lowe <chalmerlowe@google.com>
No description provided.