fix: for backwards compatibility, expose legacy retry imports#577
fix: for backwards compatibility, expose legacy retry imports#577vchudnov-g merged 9 commits intomainfrom
Conversation
5513f5e to
3a6b270
Compare
3a6b270 to
5bcd7a3
Compare
fde7bd8 to
b25579e
Compare
google/api_core/retry/__init__.py
Outdated
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
3666402 to
f7b6337
Compare
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
Fixes #575
BEGIN_COMMIT_OVERRIDE
chore: for backwards compatibility, expose legacy retry imports in the refactored retry logic
END_COMMIT_OVERRIDE