Skip to content
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

chore: add async_rest extra for async rest dependencies #701

Merged
merged 7 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/unittest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
option: ["", "_grpc_gcp", "_wo_grpc", "_with_prerelease_deps", "_with_auth_aio"]
option: ["", "_grpc_gcp", "_wo_grpc", "_w_prerelease_deps", "_w_async_rest_extra"]
python:
- "3.7"
- "3.8"
Expand Down
4 changes: 3 additions & 1 deletion google/api_core/rest_streaming_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import google.auth.aio.transport
except ImportError as e: # pragma: NO COVER
raise ImportError(
"google-auth>=2.35.0 is required to use asynchronous rest streaming."
"`google-api-core[async_rest]` is required to use asynchronous rest streaming. "
"Install the `async_rest` extra of `google-api-core` using "
"`pip install google-api-core[async_rest]`."
) from e

import google.protobuf.message
Expand Down
51 changes: 34 additions & 17 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"unit",
"unit_grpc_gcp",
"unit_wo_grpc",
"unit_with_auth_aio",
"unit_w_prerelease_deps",
"unit_w_async_rest_extra",
"cover",
"pytype",
"mypy",
Expand Down Expand Up @@ -110,7 +111,7 @@ def install_prerelease_dependencies(session, constraints_path):
session.install(*other_deps)


def default(session, install_grpc=True, prerelease=False, install_auth_aio=False):
def default(session, install_grpc=True, prerelease=False, install_async_rest=False):
Copy link
Contributor

@vchudnov-g vchudnov-g Sep 30, 2024

Choose a reason for hiding this comment

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

I suggest restructuring just slightly for brevity (I marked with ## the sections I changed). This depends on pip install foo[] with an empty extras list working, which I think it does. If not, one more line will be needed.

    install_extras = []    ##
    if install_grpc:       ##
        install_extras.append("grpc")  

    constraints_dir = str(CURRENT_DIRECTORY / "testing")
    if install_async_rest:   ##
        install_extras.append("async_rest")
        constraints_type = "async-rest-"
    else:
        constraints_type = ""

    if prerelease:
        install_prerelease_dependencies(
            session,
            f"{constraints_dir}/constraints-{constraints_type}{PYTHON_VERSIONS[0]}.txt",
        )
        # This *must* be the last install command to get the package from source.
        session.install(
            "-e", f".[{','.join(install_extras)}]", "--no-deps" ## see my other comment about installing grpc
        )
    else:
        constraints_file = (
            f"{constraints_dir}/constraints-{constraints_type}-{session.python}.txt"
        )
        # fall back to standard constraints file
        if not pathlib.Path(constraints_file).exists():
            constraints_file = f"{constraints_dir}/constraints-{session.python}.txt"

        session.install(
            "-e",
            f".[{','.join(install_extras)}]",  ##
            "-c",
            constraints_file,
        )

Copy link
Contributor

Choose a reason for hiding this comment

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

addressed.

"""Default unit test session.

This is intended to be run **without** an interpreter set, so
Expand All @@ -130,24 +131,40 @@ def default(session, install_grpc=True, prerelease=False, install_auth_aio=False
)

constraints_dir = str(CURRENT_DIRECTORY / "testing")

constraints_type = "async-rest-" if install_async_rest else ""
if prerelease:
install_prerelease_dependencies(
session, f"{constraints_dir}/constraints-{PYTHON_VERSIONS[0]}.txt"
session,
f"{constraints_dir}/constraints-{constraints_type}{PYTHON_VERSIONS[0]}.txt",
)
# This *must* be the last install command to get the package from source.
session.install("-e", ".", "--no-deps")
else:
session.install(
"-e",
".[grpc]" if install_grpc else ".",
"-c",
f"{constraints_dir}/constraints-{session.python}.txt",
"-e", "." + ("[async_rest]" if install_async_rest else ""), "--no-deps"
Copy link
Contributor

Choose a reason for hiding this comment

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

This was a pre-existing issue: at the top of the function we require that install_grpc be set if prerelease is set. But here we don't install grpc. Is that an oversight? It seems to me we probably should install it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

addressed.

)
else:
constraints_file = (
f"{constraints_dir}/constraints-{constraints_type}{session.python}.txt"
)
# fall back to standard constraints file
if not pathlib.Path(constraints_file).exists():
ohmayr marked this conversation as resolved.
Show resolved Hide resolved
constraints_file = f"{constraints_dir}/constraints-{session.python}.txt"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a reminder at the top of the else that the constraints file does not determine whether the listed imports are imported.

Copy link
Contributor

Choose a reason for hiding this comment

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

addressed.


if install_auth_aio:
session.install(
"google-auth @ git+https://git@github.com/googleapis/google-auth-library-python@8833ad6f92c3300d6645355994c7db2356bd30ad"
"-e",
"."
+ (
"[grpc,async_rest]"
if install_grpc and install_async_rest
else (
"[grpc]"
if install_grpc
else "[async_rest]"
if install_async_rest
else ""
)
),
"-c",
constraints_file,
)

# Print out package versions of dependencies
Expand Down Expand Up @@ -205,7 +222,7 @@ def unit(session):


@nox.session(python=PYTHON_VERSIONS)
def unit_with_prerelease_deps(session):
def unit_w_prerelease_deps(session):
"""Run the unit test suite."""
default(session, prerelease=True)

Expand Down Expand Up @@ -236,9 +253,9 @@ def unit_wo_grpc(session):


@nox.session(python=PYTHON_VERSIONS)
def unit_with_auth_aio(session):
"""Run the unit test suite with google.auth.aio installed"""
default(session, install_auth_aio=True)
def unit_w_async_rest_extra(session):
"""Run the unit test suite with the `async_rest` extra"""
default(session, install_async_rest=True)


@nox.session(python=DEFAULT_PYTHON_VERSION)
Expand All @@ -261,7 +278,7 @@ def mypy(session):
"""Run type-checking."""
# TODO(https://github.com/googleapis/python-api-core/issues/682):
# Use the latest version of mypy instead of mypy<1.11.0
session.install(".[grpc]", "mypy<1.11.0")
session.install(".[grpc,async_rest]", "mypy<1.11.0")
session.install(
"types-setuptools",
"types-requests",
Expand Down
3 changes: 3 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
"requests >= 2.18.0, < 3.0.0.dev0",
]
extras = {
"async_rest": [
"google-auth[aiohttp] >= 2.35.0, < 3.0.dev0",
],
"grpc": [
"grpcio >= 1.33.2, < 2.0dev",
"grpcio >= 1.49.1, < 2.0dev; python_version>='3.11'",
Expand Down
1 change: 0 additions & 1 deletion testing/constraints-3.7.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ googleapis-common-protos==1.56.2
protobuf==3.19.5
google-auth==2.14.1
requests==2.18.0
packaging==14.3
ohmayr marked this conversation as resolved.
Show resolved Hide resolved
grpcio==1.33.2
grpcio-status==1.33.2
grpcio-gcp==0.2.2
Expand Down
17 changes: 17 additions & 0 deletions testing/constraints-async-rest-3.7.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# This constraints file is used to check that lower bounds
ohmayr marked this conversation as resolved.
Show resolved Hide resolved
# are correct in setup.py
# List *all* library dependencies and extras in this file.
# Pin the version to the lower bound.
#
# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev",
# Then this file should have foo==1.14.0
googleapis-common-protos==1.56.2
protobuf==3.19.5
google-auth==2.35.0
# from google-auth[aiohttp]
aiohttp==3.6.2
requests==2.20.0
grpcio==1.33.2
grpcio-status==1.33.2
grpcio-gcp==0.2.2
proto-plus==1.22.3
7 changes: 1 addition & 6 deletions tests/asyncio/test_rest_streaming_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,9 @@

try:
from google.auth.aio.transport import Response

AUTH_AIO_INSTALLED = True
except ImportError:
AUTH_AIO_INSTALLED = False

if not AUTH_AIO_INSTALLED: # pragma: NO COVER
pytest.skip(
"google-auth>=2.35.0 is required to use asynchronous rest streaming.",
"google-api-core[async_rest] is required to test asynchronous rest streaming.",
allow_module_level=True,
)

Expand Down