Skip to content
This repository was archived by the owner on Feb 23, 2026. It is now read-only.

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

Merged
vchudnov-g merged 9 commits into from
Jan 29, 2024
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ffe7555
fix(WIP): for backwards compatibility, expose legacy retry imports
vchudnov-g Dec 20, 2023
5bcd7a3
fix: add legacy imports to preserve backwards compatibility
vchudnov-g Jan 26, 2024
b25579e
reformat as requiired by prechecks
vchudnov-g Jan 26, 2024
febe8b8
Import individually
vchudnov-g Jan 26, 2024
6e7e90b
fix: do not expose transitive imports purely for bw compatibility
vchudnov-g Jan 29, 2024
1ad808e
Merge branch 'main' into retries-include
vchudnov-g Jan 29, 2024
f7b6337
remove cruft
vchudnov-g Jan 29, 2024
7f542ec
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jan 29, 2024
821ebce
Update Copyright year
vchudnov-g Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion google/api_core/retry/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Copyright 2017 Google LLC

#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,6 +28,22 @@
from .retry_streaming_async import AsyncStreamingRetry
from .retry_streaming_async import retry_target_stream as retry_target_stream_async

# The following imports are for backwards compatibility with https://github.com/googleapis/python-api-core/blob/4d7d2edee2c108d43deb151e6e0fdceb56b73275/google/api_core/retry.py
#
# TODO: Revert these imports on the next major version release (https://github.com/googleapis/python-api-core/issues/576)
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

@vchudnov-g vchudnov-g Jan 29, 2024

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.

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.

@vchudnov-g vchudnov-g Jan 29, 2024

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.

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?

@vchudnov-g vchudnov-g Jan 29, 2024

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.

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.

from google.api_core import datetime_helpers # noqa: F401
from google.api_core import exceptions # noqa: F401
from google.auth import exceptions as auth_exceptions # noqa: F401

__all__ = (
"exponential_sleep_generator",
"if_exception_type",
Expand Down
56 changes: 56 additions & 0 deletions tests/unit/retry/test_retry_imports.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Copyright 2023 Google LLC
Comment thread
parthea marked this conversation as resolved.
Outdated
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


def test_legacy_imports_retry_unary_sync():
# TODO: Delete this test when when we revert these imports on the
# next major version release
# (https://github.com/googleapis/python-api-core/issues/576)

from google.api_core.retry import datetime # noqa: F401
from google.api_core.retry import functools # noqa: F401
from google.api_core.retry import logging # noqa: F401
from google.api_core.retry import random # noqa: F401
from google.api_core.retry import sys # noqa: F401
from google.api_core.retry import time # noqa: F401
from google.api_core.retry import inspect # noqa: F401
from google.api_core.retry import warnings # noqa: F401
from google.api_core.retry import Any # noqa: F401
from google.api_core.retry import Callable # noqa: F401
from google.api_core.retry import TypeVar # noqa: F401
from google.api_core.retry import TYPE_CHECKING # noqa: F401

from google.api_core.retry import datetime_helpers # noqa: F401
from google.api_core.retry import exceptions # noqa: F401
from google.api_core.retry import auth_exceptions # noqa: F401

### FIXME: How do we test the following, and how do we import it in __init__.py?
# import google.api_core.retry.requests.exceptions

Comment thread
parthea marked this conversation as resolved.
Outdated

def test_legacy_imports_retry_unary_async():
# TODO: Delete this test when when we revert these imports on the
# next major version release
# (https://github.com/googleapis/python-api-core/issues/576)

from google.api_core import retry_async # noqa: F401

### FIXME: each of the following cause errors on the "retry_async" part: module not found
# import google.api_core.retry_async.functools
# from google.api_core.retry_async import functools
#
## For the above, I tried the following in api_core/__init__.py
## and none made the above statements pass:
# from google.api_core.retry import retry_unary_async as retry_async
# import google.api_core.retry.retry_unary_async as retry_async