FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: for backwards compatibility, expose legacy retry imports by vchudnov-g · Pull Request #577 · googleapis/python-api-core · GitHub

This repository was archived by the owner on Feb 23, 2026. It is now read-only.
/ python-api-core Public archive

fix: for backwards compatibility, expose legacy retry imports - #577

Merged
vchudnov-g merged 9 commits into
mainfrom
retries-include
Jan 29, 2024
Merged

fix: for backwards compatibility, expose legacy retry imports#577
vchudnov-g merged 9 commits into
mainfrom
retries-include

Conversation

vchudnov-g commented Dec 20, 2023
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #575

BEGIN_COMMIT_OVERRIDE
chore: for backwards compatibility, expose legacy retry imports in the refactored retry logic
END_COMMIT_OVERRIDE

vchudnov-g requested review from a team, daniel-sanche and parthea December 20, 2023 00:06
product-auto-label Bot added the size: m Pull request size is medium. label Dec 20, 2023
vchudnov-g marked this pull request as draft December 20, 2023 00:06
Comment thread google/api_core/retry/__init__.py Outdated
Comment on lines +34 to +42
import datetime # noqa: F401
import functools # noqa: F401
import logging # noqa: F401
import random # noqa: F401
import sys # noqa: F401
import time # noqa: F401
import inspect # noqa: F401
import warnings # noqa: F401
from typing import Any, Callable, TypeVar, TYPE_CHECKING # noqa: F401

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We were having a discussion today as to whether these imports of standard libraries are appropriate. The correct thing to do is for users to import them directly, and not throughgoogle.api_core.retry. However, if they have been using these imports incorrectly (à la import google.api_core.retry.logging), then us not providing these symbols here is technically a breaking change: they'll have to change their calling code. Should we require them to do that, so that they import things correctly, or do we adhere to a strict/pedantic definition of breaking change, where the user's code really does not have to change at all, even if they are using a non-recommended pattern?

Part of the issue extends beyond just this refactoring: what does this mean for future maintenance work that happens to remove an import (import logging, let's say) but keeps our surface the same. Should we continue to expose the import we no longer need just so we don't break users who unwisely were using import google.api_core.retry.logging instead of import logging? That doesn't seem like a good idea, but breaking users still makes me uncomfortable.

I have not yet been able to find a policy document talking about this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Searches like this one on GitHub and this one on Google suggest that there aren't many repos, if any, importing standard packages in non-standard ways, so maybe we're fine to to not expose them here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

After a separate discussion, we agreed to not expose standard imports transitively if we don't need them in our implementation.

I think it would be good if the Python community spelled this out somewhere, but that's a separate issue.

product-auto-label Bot added size: s Pull request size is small. and removed size: m Pull request size is medium. labels Jan 29, 2024
vchudnov-g marked this pull request as ready for review January 29, 2024 21:20
vchudnov-g changed the title fix(WIP): for backwards compatibility, expose legacy retry imports fix: for backwards compatibility, expose legacy retry imports Jan 29, 2024
Comment thread tests/unit/retry/test_retry_imports.py Outdated
Comment thread tests/unit/retry/test_retry_imports.py Outdated
vchudnov-g merged commit cb777a1 into main Jan 29, 2024
vchudnov-g deleted the retries-include branch January 29, 2024 21:37
vchudnov-g added the release-please:force-run To run release-please label Jan 29, 2024
release-please Bot removed the release-please:force-run To run release-please label Jan 29, 2024
Floressek pushed a commit to Floressek/ImprovedRag that referenced this pull request Nov 18, 2025
Problem:
AttributeError: module 'google._upb._message' has no attribute 'MessageMapContainer'

RAGAS pulls old protobuf versions that conflict with newer proto-plus.

Solution:
Pin protobuf>=4.25.0,<6.0.0 and proto-plus>=1.24.0 to avoid conflicts.

Reference: googleapis/python-api-core#577
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

continue exposing some retry imports for backwards compatibility

2 participants


Back | FazBrowse Home | New Git URL