feat(fdc): Add execute_graphql and execute_graphql_read support with Pythonic impersonation - #970
Open
mk2023 wants to merge 21 commits into
Open
feat(fdc): Add execute_graphql and execute_graphql_read support with Pythonic impersonation#970mk2023 wants to merge 21 commits into
mk2023 wants to merge 21 commits into
Conversation
Implemented _make_gql_request on _DataConnectApiClient to execute and handle responses/errors for GraphQL operations. Added corresponding unit tests in tests/test_data_connect.py.
Refactored _parse_graphql_response and added robust recursive type deserialization to _DataConnectApiClient. - Implemented _deserialize_type and _deserialize_dataclass helper methods to support nested dataclasses, generic lists (List[T]), generic dictionaries (Dict[K, V]), Unions (Union[...]), Enums, and primitive casting. - Enhanced _make_gql_request error handling to prevent silent error swallowing when the errors key is present. - Added comprehensive unit test coverage in tests/test_data_connect.py.
…helper - Introduced QueryError subclass of FirebaseError for Data Connect GraphQL query/mutation errors and exposed it in __all__. - Extracted _check_graphql_errors helper method on _DataConnectApiClient. - Updated error handling for non-dictionary response payloads in _parse_graphql_response to raise InternalError. - Note: Did not edit parse_graphql_response because we are waiting on whether this will even be a function or not.
…nt instantiation - Removed output deserialization helpers (_extract_actual_type, _deserialize_type, _deserialize_dataclass) to return raw JSON payload dictionaries (ExecuteGraphqlResponse.data), aligning Data Connect with Firestore and Realtime Database patterns for user-defined schemas. - Updated DataConnect.__init__ to immediately instantiate _DataConnectApiClient for consistency with Node.js and other Python Admin SDK services. - Updated test suite in tests/test_data_connect.py to cover raw response parsing and immediate client instantiation.
Added execute_graphql and execute_graphql_read method signatures and docstrings to DataConnect and _DataConnectApiClient. Also introduced a comprehensive integration test suite in integration/test_data_connect.py translated from Node.js Admin SDK integration tests.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces execute_graphql and execute_graphql_read methods to the DataConnect client, along with corresponding integration and unit tests. The review feedback highlights that several of these new methods are left unimplemented (raising NotImplementedError) and provides their implementation details. Additionally, the reviewer identifies a critical type-checking bug in the validation logic when variables_type is Any, and points out a mismatch in a test assertion message.
stephenarosaj
requested changes
Jul 28, 2026
stephenarosaj
left a comment
stephenarosaj
left a comment
There was a problem hiding this comment.
in progress, leaving comments early - have to look at test cases still
Added an explicit __init__ constructor to Impersonation to validate parameter configurations at object instantiation time. Enforced choosing either unauthenticated=True or auth_claims, with support for both auth_claims (snake_case) and authClaims (camelCase). Updated class docstring to recommend factory methods. Also added unit test suite TestImpersonation in tests/test_data_connect.py.
mk2023
marked this pull request as ready for review
July 29, 2026 07:01
…mulator test setup Implemented execute_graphql and execute_graphql_read methods on DataConnect and _DataConnectApiClient. Configured Data Connect emulator schema, connector, queries, mutations, seed script, and GitHub Actions CI workflow step.
stephenarosaj
approved these changes
Jul 31, 2026
stephenarosaj
left a comment
stephenarosaj
left a comment
There was a problem hiding this comment.
LGTM with some changes requested - mostly small stuff, only a few blocking comments
stephenarosaj
requested changes
Jul 31, 2026
stephenarosaj
left a comment
stephenarosaj
left a comment
There was a problem hiding this comment.
Accidentally selected Approve but mean Request changes - re-submitting review
LGTM with some changes requested - mostly small stuff, only a few blocking comments
…d streamlined read integration tests
stephenarosaj
requested changes
Jul 31, 2026
stephenarosaj
requested changes
Jul 31, 2026
…leanup - Impersonation API: Updated Impersonation to use auth_claims (snake_case) in Python land, while translating it to authClaims (camelCase) in _prepare_graphql_payload during JSON wire serialization. - Impersonation Validation: Updated _validate_impersonation_options to validate auth_claims in Python land. - Integration Tests (integration/test_data_connect.py): Added UPDATED_FRED_EMAIL mutation test fixtures with real state changes, restored initial state via UPSERT_FRED_EMAIL cleanup at the end of mutation tests, reordered query tests before mutations, and removed redundant read impersonation test cases.
mk2023
changed the title
itsrakhil
approved these changes
Aug 4, 2026
jonathanedey
requested changes
Aug 4, 2026
jonathanedey
left a comment
jonathanedey
left a comment
Contributor
There was a problem hiding this comment.
Thanks @mk2023! This overall looks great, with a few comments mainly on testing! I've also added the integration test tag so those tests should run from your next commit.
…t suite - Type Annotations: Added from __future__ import annotations to dataconnect.py for clean return type hints. - Integration Test Fixtures: Added setup_and_cleanup_database fixture in integration/test_data_connect.py using raw HTTP bash scripts (seed.sh and cleanup.sh) before/after every test without relying on the SDK under test. - Emulator Cleanup: Added integration/emulators/cleanup.sh script executing raw HTTP POST deleteMany mutations via curl. - Test Naming & CI: Updated TestImpersonation test method names to start with test_impersonation_ and removed unnecessary credentials flag from ci.yml.
… CI collision Updated default_app fixture in integration/test_data_connect.py to safely delete pre-existing default app before initializing with EmulatorAdminCredentials, allowing dataconnect.client() to implicitly use default_app.
…ta_connect.py
Aligned integration/test_data_connect.py with test_functions.py and test_db.py by using a named app fixture ('integration-dataconnect'), overriding default_app with pass, and injecting dc_client fixture into test methods.
…ot set Added a check_emulator autouse module fixture in integration/test_data_connect.py. Unlike services with dynamic resource creation (e.g., Realtime Database), Data Connect requires a pre-deployed Cloud SQL Postgres schema and connector. Since live integration projects do not host these resources, Data Connect integration tests are strictly emulator-only and are safely skipped when DATA_CONNECT_EMULATOR_HOST is absent.
…ta_connect.py Removed check_emulator fixture from integration/test_data_connect.py. Note that stage_release integration test runs are expected to fail until the expected GraphQL schema and connector are deployed to the FDC service within the GCP project tied to the service key.
stephenarosaj
requested changes
Aug 5, 2026
…sh inside dataconnect/ Combined seed.sh and cleanup.sh into a unified setup_teardown.sh script located in integration/emulators/dataconnect/ to clarify its exclusive use for FDC, eliminate code duplication, and add support for both emulator and live GCP testing.
…cript Updated setup_teardown.sh to dynamically resolve project IDs and credentials, supporting both the local emulator and live GCP production endpoints.
jonathanedey
reviewed
Aug 5, 2026
…ixture Updated setup_and_cleanup_database in integration/test_data_connect.py to use dc_client.execute_graphql directly for seeding and teardown, removing external shell scripts.
jonathanedey
approved these changes
Aug 5, 2026
jonathanedey
left a comment
jonathanedey
left a comment
Contributor
There was a problem hiding this comment.
LGTM Thanks!
I'll leave the option open for triggering the integration tests (just let me know and I can re-add the tag) but this looks good to merge to wine from Admin SDK side.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
✨ Adds public
execute_graphqlandexecute_graphql_readmethods toDataConnect, refactorsImpersonationto use Pythonicauth_claims(PEP 8 snake_case) in Python land while translating toauthClaimsduring JSON payload serialization, and includes comprehensive unit and emulator integration test suites.Highlights
✨ Key changes:
firebase_admin.dataconnect): Addedexecute_graphqlfor query/mutation execution andexecute_graphql_readfor read-only queries with mutation validation.Impersonationto storeauth_claimsin Python land, with automaticauth_claims->authClaimskey translation in_prepare_graphql_payloadfor network serialization._validate_impersonation_optionsto validateauth_claimsin Python land.tests/test_data_connect.py): Added 82 unit tests covering method execution, payload serialization, dataclass variables, impersonation options, and error parsing.integration/): Added 20 integration tests against the Data Connect emulator, includingseed.shdata seeding (fred_id,jeff_id,email_id) and state cleanup (UPSERT_FRED_EMAIL)..github/workflows/ci.yml): Added--certflag and emulator seeding step for CI integration testing.Detailed Changelog
Core SDK Changes
firebase_admin/dataconnect.py:execute_graphqlandexecute_graphql_readonDataConnect.Impersonationclass withunauthenticated()andauthenticated(auth_claims=...)._prepare_graphql_payloadto translateauth_claimstoauthClaimsvia dictionary key popping._validate_impersonation_optionsandExecuteGraphqlResponsedataclass.Integration & Emulator Changes
integration/test_data_connect.py: Added 20 integration tests covering queries, mutations, multi-operation documents, impersonated requests, and mutation state cleanup.integration/emulators/seed.sh: Added script to seed initial test data (fred_id,jeff_id,email_id) into the emulator..github/workflows/ci.yml: Added--certflag and emulator seeding to integration test workflow.Unit Test Changes
tests/test_data_connect.py: Added 82 unit tests, refactoring shared constants (TEST_URL,TEST_HEADERS,TEST_PAYLOAD,TEST_AUTH_CLAIMS,TEST_VARIABLES) and dataclasses (UserProfile,CreateUserVariables,User).Testing Strategy
pytest tests/test_data_connect.py(82/82 PASSED)DATA_CONNECT_EMULATOR_HOST=127.0.0.1:9399 pytest integration/test_data_connect.py --cert tests/data/service_account.json(20/20 PASSED)./lint.shContext Sources Used: