googleapis
x-goog-request-params is not sending the fully qualified path to enum values
PR #1442 CI is failing in test_repeat_data_simple_path_field_headers because the field path to the enum value does not match between the actual value and the test expectation. We need to decide which is right, and make them match
More details:
You can see the error here.
I also set up an explanatory commit in that PR, to add output to the test. You can see the output locally by cloning the PR at that commit, running nox showcase_unit-3.8, for example, and looking for "FIXME" in the output.
You'll see that the the test expects info.f_kingdom=compliance.ComplianceData.LifeKingdom.ARCHAEBACTERIA but the actual mock call returns info.f_kingdom=LifeKingdom.ARCHAEBACTERIA.
I assume the fully-qualified field path in the expectation is correct, but we should check.
I am marking this as a P1 for now because getting the field headers correct is arguably important for REGAPIC GA launch.
EDIT (2022-10-05): This issue manifests itself in gRPC tests, not REST tests. That said, we probably need to have similar tests for REST (cf #1466)
I wonder whether we can get away with always sending the enum values as numbers in this header, regardless of the numeric enum feature. I'm inquiring internally.
Hi Folks, any updates on this? It triggered OO-SLO and seems like a blocker for REGAPIC GA, if I understand correctly?
x-goog-request-params is needed by grpc transport to mimic url path, which is not present in grcp transport. In rest, routing on the server side relies on the ulr path itself, thus making x-goog-request-params irrelevant. Downgrading this to P2, but I would consider just not sending the header altogether in rest transport. Note, for example in Java REGAPIC, the x-goog-request-params logic is in grpc-specific dependencies, and is not even loaded in memory for rest-only clients.
@vam-google I'm not sure the header is completely irrelevant. If no overrides are presented, then I agree we could omit it. However, if the new annotation google.api.routing (AIP 4222) is used (example), then I believe we need to include x-goog-request-params as specified. I'm not sure yet whether the Google use cases make this a P1. More discussion in the internal email thread.
OK, after more analysis, I see this error is coming from the function test_{{ method_name }}_field_headers() function defined here inside the grpc_required_tests macro. It appears that this gets called only when gRPC transport is generated, and only applies the test to that transport.
That means there are two issues here:
- We need to fix this at least for
transport=grpc: enums in thex-goog-request-paramsare not matching the values we mock. Probably the fully-qualified values are correct, but I'm inquiring internally. Alternately, maybe we can just use numeric values over gRPC. (Over REST, maybe we only use numeric value if the numeric enums setting is on? But for compute, we're can't use the synthetic numeric values, and the fully qualified value may be confusing to the server....) - As per my previous comment, and subject to confirmation, we probably do need to test
x-goog-request-paramswhentransport=rest, at least whengoogle.api.routingappears in the protos. Created #1466 to track this.
As per the internal email discussion (you're included in the thread, @vam-google), we're expected to include x-goog-request-params even in the REST case no matter what, even if there's no google.api.routing. Something to keep in mind for Java.
This issue also appears with //google/devtools/testing/v1
See test results below:
(py39) partheniou@partheniou-vm-3:~/git/googleapis$ bazel run //google/devtools/testing/v1:testing_py_gapic_test
INFO: Analyzed target //google/devtools/testing/v1:testing_py_gapic_test (1 packages loaded, 13 targets configured).
INFO: Found 1 target...
Target //google/devtools/testing/v1:testing_py_gapic_test up-to-date:
bazel-bin/google/devtools/testing/v1/testing_py_gapic_pytest.py
bazel-bin/google/devtools/testing/v1/testing_py_gapic_test.py
bazel-bin/google/devtools/testing/v1/testing_py_gapic_test
INFO: Elapsed time: 0.272s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: external/bazel_tools/tools/test/test-setup.sh google/devtools/testing/v1/testing_py_gapic_test
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //google/devtools/testing/v1:testing_py_gapic_test
-----------------------------------------------------------------------------
........................................................................ [ 22%]
...........................................................FF........... [ 45%]
........................................................................ [ 68%]
........................................................................ [ 91%]
........................... [100%]
=================================== FAILURES ===================================
_______________ test_get_test_environment_catalog_field_headers ________________
def test_get_test_environment_catalog_field_headers():
client = TestEnvironmentDiscoveryServiceClient(
credentials=ga_credentials.AnonymousCredentials(),
)
# Any value that is part of the HTTP/1.1 URI should be sent as
# a field header. Set these to a non-empty value.
request = test_environment_discovery.GetTestEnvironmentCatalogRequest()
request.environment_type = test_environment_discovery.GetTestEnvironmentCatalogRequest.EnvironmentType.ANDROID
# Mock the actual call within the gRPC stub, and fake the request.
with mock.patch.object(
type(client.transport.get_test_environment_catalog),
'__call__') as call:
call.return_value = test_environment_discovery.TestEnvironmentCatalog()
client.get_test_environment_catalog(request)
# Establish that the underlying gRPC stub method was called.
assert len(call.mock_calls) == 1
_, args, _ = call.mock_calls[0]
assert args[0] == request
# Establish that the field header was sent.
_, _, kw = call.mock_calls[0]
> assert (
'x-goog-request-params',
'environment_type=test_environment_discovery.GetTestEnvironmentCatalogRequest.EnvironmentType.ANDROID',
) in kw['metadata']
E AssertionError: assert ('x-goog-request-params', 'environment_type=test_environment_discovery.GetTestEnvironmentCatalogRequest.EnvironmentType.ANDROID') in [('x-goog-request-params', 'environment_type=EnvironmentType.ANDROID'), ('x-goog-api-client', 'gl-python/3.9.16 grpc/1.55.1 gax/2.11.1 gapic/0.0.0')]
google/devtools/testing/v1/testing_py_gapic_srcjar.py/tests/unit/gapic/testing_v1/test_test_environment_discovery_service.py:650: AssertionError
____________ test_get_test_environment_catalog_field_headers_async _____________
@pytest.mark.asyncio
async def test_get_test_environment_catalog_field_headers_async():
client = TestEnvironmentDiscoveryServiceAsyncClient(
credentials=ga_credentials.AnonymousCredentials(),
)
# Any value that is part of the HTTP/1.1 URI should be sent as
# a field header. Set these to a non-empty value.
request = test_environment_discovery.GetTestEnvironmentCatalogRequest()
request.environment_type = test_environment_discovery.GetTestEnvironmentCatalogRequest.EnvironmentType.ANDROID
# Mock the actual call within the gRPC stub, and fake the request.
with mock.patch.object(
type(client.transport.get_test_environment_catalog),
'__call__') as call:
call.return_value = grpc_helpers_async.FakeUnaryUnaryCall(test_environment_discovery.TestEnvironmentCatalog())
await client.get_test_environment_catalog(request)
# Establish that the underlying gRPC stub method was called.
assert len(call.mock_calls)
_, args, _ = call.mock_calls[0]
assert args[0] == request
# Establish that the field header was sent.
_, _, kw = call.mock_calls[0]
> assert (
'x-goog-request-params',
'environment_type=test_environment_discovery.GetTestEnvironmentCatalogRequest.EnvironmentType.ANDROID',
) in kw['metadata']
E AssertionError: assert ('x-goog-request-params', 'environment_type=test_environment_discovery.GetTestEnvironmentCatalogRequest.EnvironmentType.ANDROID') in [('x-goog-request-params', 'environment_type=EnvironmentType.ANDROID'), ('x-goog-api-client', 'gl-python/3.9.16 grpc/1.55.1 gax/2.11.1 gapic/0.0.0')]
google/devtools/testing/v1/testing_py_gapic_srcjar.py/tests/unit/gapic/testing_v1/test_test_environment_discovery_service.py:682: AssertionError
=========================== short test summary info ============================
FAILED google/devtools/testing/v1/testing_py_gapic_srcjar.py/tests/unit/gapic/testing_v1/test_test_environment_discovery_service.py::test_get_test_environment_catalog_field_headers
FAILED google/devtools/testing/v1/testing_py_gapic_srcjar.py/tests/unit/gapic/testing_v1/test_test_environment_discovery_service.py::test_get_test_environment_catalog_field_headers_async
2 failed, 313 passed, 12 warnings in 2.60s
