feat(nl): add dynamic graph fallback for un-cached topic and peer group members - #6590
feat(nl): add dynamic graph fallback for un-cached topic and peer group members#6590clincoln8 wants to merge 2 commits into
Conversation
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 a dynamic fallback mechanism for the Data Commons Platform to handle custom and UN topics that are not present in the static 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 the 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 counterproductive. 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 introduces fallback mechanisms in topic.py to dynamically fetch member and parent properties when they are missing from the TOPIC_CACHE. It also updates _prop_val_ordered to properly split and deduplicate ordered property values, and adds comprehensive unit tests to verify these fallback behaviors. The review feedback suggests deduplicating the list of missing nodes before making API calls in _members_raw and _parents_raw to prevent redundant network requests.
| if missing_nodes: | ||
| raw_res = fetch.raw_property_values(nodes=missing_nodes, prop=prop) | ||
| for n in missing_nodes: | ||
| val_map[n] = raw_res.get(n, []) |
There was a problem hiding this comment.
To avoid redundant API calls and save network bandwidth, we should deduplicate missing_nodes before passing it to fetch.raw_property_values.
| if missing_nodes: | |
| raw_res = fetch.raw_property_values(nodes=missing_nodes, prop=prop) | |
| for n in missing_nodes: | |
| val_map[n] = raw_res.get(n, []) | |
| if missing_nodes: | |
| raw_res = fetch.raw_property_values(nodes=list(dict.fromkeys(missing_nodes)), prop=prop) | |
| for n in missing_nodes: | |
| val_map[n] = raw_res.get(n, []) |
| if missing_nodes: | ||
| parents = fetch.raw_property_values(nodes=missing_nodes, | ||
| prop=prop, | ||
| out=False) |
There was a problem hiding this comment.
Similarly, we should deduplicate missing_nodes here to prevent redundant API requests when fetching parent properties.
| if missing_nodes: | |
| parents = fetch.raw_property_values(nodes=missing_nodes, | |
| prop=prop, | |
| out=False) | |
| if missing_nodes: | |
| parents = fetch.raw_property_values(nodes=list(dict.fromkeys(missing_nodes)), | |
| prop=prop, | |
| out=False) |
Summary
In Data Commons Platform (DCP), custom datasets and UN topics are loaded dynamically into Spanner without generating static
topic_cache.jsonfiles.Previously,
_members()inserver/lib/nl/common/topic.pyonly checkedTOPIC_CACHE. When a dynamic custom or UN topic (e.g.undata/topic/theme/S73) was queried,get_members()returned empty, causing dynamic topic traversal in Explore to terminate prematurely without charts.Changes
_members(): When a topic is missing fromTOPIC_CACHE(or returns empty), fall back to calling_prop_val_ordered(node, prop + 'List')._members_raw()and_parents_raw(): Batch-fetch missing nodes from the graph viafetch.raw_property_values()._prop_val_ordered(): Iterate over all list strings insv_listacross provenances with set deduplication.server/tests/lib/nl/topic_test.py.Tested with
pytest server/tests/lib/nl/topic_test.py(5 passed) and fullpytest server/tests/lib/nl/(342 passed).