feat: Add attempt_direct_path argument to create_channel by parthea · Pull Request #583 · googleapis/python-api-core · GitHub

Fixes #482
b/267782870

This is looking good. I only reviewed the sync file and not the tests yet; I'll pick this back up tomorrow, but feel free to start responding to the comments here (or wait if you prefer; all good).

Since this is public code, I would prefer to a public (GitHub) issue in this TODO.

Fixed in e9199a0

Do we need to spell out what GCE stands for in public docs? Not everyone looking at this file may be using GCE.

Fixed in e7e3cf7

Suggested change

      
                        set to `True` will result in `ValueError` as it is not yet supported.
          
                        set to `True` will result in `ValueError` as this combination is not yet supported.

Fixed in e7e3cf7

"Should only be set in a GCE environment" is confusing in light of "Outside of GCE, the request may fall back". Clarify. I suspect you mean something like this:

Suggested change

      
                    attempt_direct_path (Optional[bool]): If set, Direct Path will be attempted when
          
                        the request is made. Direct Path provides a proxyless connection which
          
                        increases the available throughput, reduces latency, and increases
          
                        reliability. Outside of GCE, the direct path request may fallback
          
                        to DNS if this is configured by the Service. This argument should only
          
                        be set in a GCE environment and for Services that are known to support Direct Path.
          
                        If a `ServiceUnavailable` response is received when the request is sent, it is
          
                        recommended that the client repeat the request with `attempt_direct_path` set to `False`
          
                        as the Service may not support Direct Path. Using `ssl_credentials` with `attempt_direct_path`
          
                        set to `True` will result in `ValueError` as it is not yet supported.
          
                    attempt_direct_path (Optional[bool]): If set, Direct Path will be attempted when
          
                        the request is made. Direct Path is only available within a Google Compute
          
                        Engine environment and provides a proxyless connection which increases the
          
                        available throughput, reduces latency, and increases reliability. 
          
                        - This argument should only
          
                        be set in a GCE environment and for Services that are known to support Direct
          
                        Path.
          
                        - If this argument is set outside of GCE, then this request will fail 
          
                        unless the back-end service happens to have configured fall-back to DNS.       
          
                        - If the request causes a `ServiceUnavailable` response, we
          
                        recommend that the client repeat the request with `attempt_direct_path` set to
          
                        `False` as the Service may not support Direct Path. 
          
                        - Using `ssl_credentials` with `attempt_direct_path`
          
                        set to `True` will result in `ValueError` as this combination  is not yet
          
                        supported.

(And similarly for the async version)

WDYT?

Fixed in e7e3cf7

Suggested change

      
                        reliability. Outside of GCE, the direct path request may fallback
          
                        reliability. Outside of GCE, the direct path request may fall back

but see comment below

Fixed in e7e3cf7

Suggested change

      
                    target (str): The target service address which is converted into a format compatible with Direct Path.
          
                        If the target contains `dns:///` or does not have contain `:///`, the target will be converted in
          
                        a format compatible with Direct Path, otherwise the original target will be returned.
          
                    target (str): The target service address which is converted into a format compatible with Direct Path.
          
                        If the target contains `dns:///` or does not contain `:///`, the target will be converted in
          
                        a format compatible with Direct Path; otherwise the original target will be returned.

So the idea is that a :/// (except for dns:///) already denotes Direct Path, so no action is needed, correct? It might be good to be explicit about this assumption.

Done in 44d5845

It might be helpful to clarify in a comment what the dns_prefix means, which I take it is to be explicit about an endpoint living in the Internet (ie outside GCP).

Fixed in e6ea7ec

Suggested change

      
                attempt_direct_path: Optional[bool] = None,
          
                attempt_direct_path: Optional[bool] = False,

Since it's meant to be a Boolean, might as well.

Fixed in 0f3a0e4

LGTM! Nothing blocking, but I think some of my suggestions can make things tighter/clearer.

Could we simplify to just this?:

Suggested change

      
                if ssl_credentials is not None and attempt_direct_path:
          
                if ssl_credentials and attempt_direct_path:

Fixed in 60b8acb

For consistency, the _LOGGER.debug in the previous lines should probably also become a warnings.warn

Fixed in 1a7578d and d96499f

Suggested change

      
                    target (str): The target service address in the format 'hostname:port', 'dns://hostname' or other
          
                    target (str): The target service address in the format 'hostname[:port]', 'dns://hostname[:port]' or other

Fixed in af3a21b

"other compatible format": link to a place that lists compatible formats, or remove this phrase which I think is puzzling by itself.

Removed in af3a21b

Suggested change

      
                direct_path_prefix = ":///"
          
                if direct_path_prefix not in target:
          
                direct_path_separator = ":///"
          
                if direct_path_separator not in target:

Fixed in c0052e6

Q: In general, an API endpoint could depend on the URL path and not just the host+port. Is this true of Google APIs? If so, where do we deal with the path part of the URI? (I realize these functions specify only host+port as inputs, so it's clear what they expect and they're doing the right thing, but I was wondering about this more general question.)

If so, where do we deal with the path part of the URI?

I believe this is part of the transcode method

Transcodes a grpc request pattern into a proper HTTP request following the rules outlined here,

python-api-core/google/api_core/path_template.py

Lines 250 to 259 in b72929f

    
           def transcode(http_options, message=None, **request_kwargs): 
        
               """Transcodes a grpc request pattern into a proper HTTP request following the rules outlined here, 
        
               https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L44-L312 
        
                Args: 
        
                    http_options (list(dict)): A list of dicts which consist of these keys, 
        
                        'method'    (str): The http method 
        
                        'uri'       (str): The path template 
        
                        'body'      (str): The body field name (optional) 
        
                        (This is a simplified representation of the proto option `google.api.http`)

https://github.com/googleapis/gapic-generator-python/blob/b4e6cfc0be8594e2575910699389600f5e92e552/gapic/templates/%25namespace/%25name_%25version/%25sub/services/%25service/transports/rest.py.j2#L372

General notes:

The comments from the sycn version apply here too
Just a note for the future (not blocking this PR): a lot of the code seems identical or very similar to the sync code. We should consolidate sensibly to eliminate duplication. (I just filed #593 to track more such instances)

Thanks for opening that issue! I also made changes to the async code based on the sync code feedback.

default is too generic a name. I assume this is meant to take the google.auth.default you have patched in the decorators. Is there any way this parameter name could be made more descriptive (like auth-default, say), or does @mock preclude that?

(I realize this was pre-existing, so no need to spend too much time on this. But if it's easy...)

Fixed in d63402e

Would it be worth making this test function take a target parameter, and then parametrizing it as we do for test_create_channel_implicit_with_default_host below? target seems to be used the same way in both places, and this would allow us to check here the dns:/// and another-c2p:/// cases you parametrize below.

Done in 396d8fa

Same comments as in sync version apply here.

Done in 396d8fa

product-auto-label Bot added the size: m Pull request size is medium. label Jan 26, 2024

parthea force-pushed the add-support-for-direct-path branch 5 times, most recently from 676ca58 to e777b53 Compare January 31, 2024 00:56

product-auto-label Bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Jan 31, 2024

parthea force-pushed the add-support-for-direct-path branch from e777b53 to 0e974b6 Compare January 31, 2024 01:09

parthea mentioned this pull request Jan 12, 2026

Add support for using ssl_credentials and attempt_direct_path in google.api_core.grpc_helpers.create_channel googleapis/google-cloud-python#15061

Closed

parthea force-pushed the add-support-for-direct-path branch 2 times, most recently from c4f537d to 257d4df Compare January 31, 2024 01:22

feat: Add attempt_direct_path argument to create_channel

421becd

parthea force-pushed the add-support-for-direct-path branch from 1d40546 to 421becd Compare January 31, 2024 01:28

parthea added 2 commits January 30, 2024 20:28

Merge branch 'main' into add-support-for-direct-path

65b994e

add more test cases

b1010ea

parthea marked this pull request as ready for review January 31, 2024 01:33

parthea requested review from a team January 31, 2024 01:33

parthea assigned vchudnov-g Jan 31, 2024

parthea added 2 commits January 31, 2024 20:03

fix docstring

9a1957a

fix docstring

b223b85

vchudnov-g reviewed Feb 1, 2024

View reviewed changes

vchudnov-g approved these changes Feb 1, 2024

View reviewed changes

parthea added 6 commits February 4, 2024 14:54

update docstring of attempt_direct_path arg

e7e3cf7

update docstring of target arg

44d5845

Add comment for dns_prefix local variable

e6ea7ec

Set the default value of attempt_direct_path to False

0f3a0e4

simplify conditional statement

60b8acb

use warnings.warn instead of _LOGGER.debug

1a7578d

parthea added 4 commits February 4, 2024 15:06

update docstring of target arg in _modify_target_for_direct_path

af3a21b

s/direct_path_prefix/direct_path_separator

c0052e6

default->google_auth_default

d63402e

parametrize target in def test_create_channel_implicit

396d8fa

parthea force-pushed the add-support-for-direct-path branch from b4a6edf to 396d8fa Compare February 4, 2024 17:48

Merge branch 'main' into add-support-for-direct-path

d3598ee

parthea mentioned this pull request Feb 4, 2024

Add ability to detect Google Compute Engine Environment in _create_composite_credentials #598

Closed

parthea added 3 commits February 4, 2024 17:53

Add github issue for TODO

e9199a0

filter deprecation warning related to grpcio-gcp

d96499f

format docstring

d41ca10

parthea merged commit 94726e7 into main Feb 4, 2024

parthea deleted the add-support-for-direct-path branch February 4, 2024 20:00

release-please Bot mentioned this pull request Feb 4, 2024

chore(main): release 2.17.0 #599

Merged

This was referenced May 30, 2025

chore(main): release 2.0.0 #822

Closed

chore(main): release 2.0.0 #823

Closed

+                  else:
+                      # Use grpc.compute_engine_channel_credentials in order to support Direct Path.
+                      # See https://grpc.github.io/grpc/python/grpc.html#grpc.compute_engine_channel_credentials
+                      # TODO(b/323073050): Although `grpc.compute_engine_channel_credentials`

                   default_scopes=None,
                   default_host=None,
                   compression=None,
+                  attempt_direct_path: Optional[bool] = None,

+                  # If `ssl_credentials` is set and `attempt_direct_path` is set to `True`,
+                  # raise ValueError as this is not yet supported.
+                  # See https://github.com/googleapis/python-api-core/issues/590
+                  if ssl_credentials is not None and attempt_direct_path:

-              def test_create_channel_implicit(grpc_secure_channel, default, composite_creds_call):
+              def test_create_channel_implicit(
+                  grpc_secure_channel,
+                  default,

	set to `True` will result in `ValueError` as it is not yet supported.
	set to `True` will result in `ValueError` as this combination is not yet supported.

	reliability. Outside of GCE, the direct path request may fallback
	reliability. Outside of GCE, the direct path request may fall back

	attempt_direct_path: Optional[bool] = None,
	attempt_direct_path: Optional[bool] = False,

	if ssl_credentials is not None and attempt_direct_path:
	if ssl_credentials and attempt_direct_path:

                               "Compression argument is being ignored for grpc_gcp.secure_channel creation."
                           )
+                      if attempt_direct_path:
+                          warnings.warn(

	target (str): The target service address in the format 'hostname:port', 'dns://hostname' or other
	target (str): The target service address in the format 'hostname[:port]', 'dns://hostname[:port]' or other

		direct_path_prefix = ":///"
		if direct_path_prefix not in target:

	def transcode(http_options, message=None, **request_kwargs):
	"""Transcodes a grpc request pattern into a proper HTTP request following the rules outlined here,
	https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L44-L312

	Args:
	http_options (list(dict)): A list of dicts which consist of these keys,
	'method' (str): The http method
	'uri' (str): The path template
	'body' (str): The body field name (optional)
	(This is a simplified representation of the proto option `google.api.http`)

               import functools
-              from typing import Generic, Iterator, AsyncGenerator, TypeVar
+              from typing import AsyncGenerator, Generic, Iterator, Optional, TypeVar



		@mock.patch("grpc.composite_channel_credentials")
		@pytest.mark.parametrize(

		)


		def _modify_target_for_direct_path(target: str) -> str:

feat: Add attempt_direct_path argument to create_channel by parthea · Pull Request #583 · googleapis/python-api-core · GitHub

Uh oh!

feat: Add attempt_direct_path argument to create_channel - #583

feat: Add attempt_direct_path argument to create_channel#583parthea merged 19 commits intomaingoogleapis/python-api-core:mainfrom add-support-for-direct-pathgoogleapis/python-api-core:add-support-for-direct-pathCopy head branch name to clipboard

Conversation

parthea commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vchudnov-g left a comment

Choose a reason for hiding this comment

Uh oh!

vchudnov-g Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vchudnov-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feat: Add attempt_direct_path argument to create_channel#583
parthea merged 19 commits into
mainfrom
add-support-for-direct-path

parthea commented Jan 26, 2024 •
edited
Loading

vchudnov-g Jan 31, 2024 •
edited
Loading