chore: Use referenced_property_graphs api to get property graph name for schema view#16174
chore: Use referenced_property_graphs api to get property graph name for schema view#16174ericfe-google wants to merge 8 commits intomainfrom
Conversation
…used from_string method
…graphs and fix tests
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant improvements to how BigQuery property graphs are identified and managed within the BigQuery client library and BigQuery Magics. By adding a dedicated Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enhances the BigQuery client libraries to support Property Graph queries. It introduces a new PropertyGraphReference class and adds a referenced_property_graphs property to the QueryJob object, allowing retrieval of property graph metadata from query job statistics. The bigquery-magics package is updated to utilize this new client library functionality, replacing manual parsing of GRAPH queries with the more robust referenced_property_graphs property. This includes refactoring the _get_graph_schema function and updating/adding several unit tests to cover the new logic. Additionally, some test assertions for user agents were updated to use startswith, and a retry mechanism initialization was modified in a test. A review comment suggests improving the mocking strategy in some tests by using mock.create_autospec and directly manipulating the _properties dictionary for QueryJob to enhance test robustness.
| query_job = mock.Mock() | ||
| query_job.configuration.destination.project = "p" | ||
| query_job.configuration.destination.dataset_id = "d" | ||
| query_job.configuration.destination.table_id = "t" | ||
|
|
||
| graph_ref = mock.Mock() | ||
| graph_ref.project = "p" | ||
| graph_ref.dataset_id = "my_dataset" | ||
| graph_ref.property_graph_id = "my_graph" | ||
| query_job.referenced_property_graphs = [graph_ref] |
There was a problem hiding this comment.
Using mock.Mock() is less safe than mock.create_autospec as it doesn't enforce the mocked object's interface. It's better to stick with create_autospec for better test robustness. You can mock the referenced_property_graphs property by setting the underlying _properties dictionary, which is how the property gets its data. This is more aligned with how the actual QueryJob object works.
This feedback also applies to test_add_graph_widget_no_graph_name and test_add_graph_widget_schema_not_found.
| query_job = mock.Mock() | |
| query_job.configuration.destination.project = "p" | |
| query_job.configuration.destination.dataset_id = "d" | |
| query_job.configuration.destination.table_id = "t" | |
| graph_ref = mock.Mock() | |
| graph_ref.project = "p" | |
| graph_ref.dataset_id = "my_dataset" | |
| graph_ref.property_graph_id = "my_graph" | |
| query_job.referenced_property_graphs = [graph_ref] | |
| query_job = mock.create_autospec(bigquery.job.QueryJob, instance=True) | |
| query_job.configuration.destination.project = "p" | |
| query_job.configuration.destination.dataset_id = "d" | |
| query_job.configuration.destination.table_id = "t" | |
| graph_ref_resource = { | |
| "projectId": "p", | |
| "datasetId": "my_dataset", | |
| "propertyGraphId": "my_graph", | |
| } | |
| query_job._properties = { | |
| "statistics": { | |
| "query": {"referencedPropertyGraphs": [graph_ref_resource]} | |
| } | |
| } |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕