feat(cli): add sdmx query commands#175
Conversation
There was a problem hiding this comment.
Code Review
This pull request renames the namespace variable to instance_name across Terraform modules and the admin CLI, while maintaining backward compatibility. It also introduces a new sdmx command group to query custom SDMX v3 observation APIs and enhances tf_utils.py to support fetching Terraform outputs directly from GCS remote state. Feedback on these changes focuses on improving the manual ADC loading logic for Windows compatibility and standard precedence, initializing the GCS client with the target project ID for robustness, and refining error handling when parsing non-OK HTTP responses.
| adc_path = os.path.expanduser("~/.config/gcloud/application_default_credentials.json") | ||
| if os.path.exists(adc_path): | ||
| try: | ||
| with open(adc_path) as f: | ||
| data = json.load(f) | ||
| if data.get("type") == "authorized_user": | ||
| payload = { | ||
| "client_id": data["client_id"], | ||
| "client_secret": data["client_secret"], | ||
| "grant_type": "refresh_token", | ||
| "refresh_token": data["refresh_token"], | ||
| } | ||
| res = requests.post("https://oauth2.googleapis.com/token", data=payload, timeout=10) | ||
| if res.ok: | ||
| id_token_val = res.json().get("id_token") | ||
| if id_token_val: | ||
| session = requests.Session() | ||
| session.headers.update({"Authorization": f"Bearer {id_token_val}"}) | ||
| return session | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
The manual ADC loading logic has two issues:
- ADC Precedence: It bypasses standard Google Cloud ADC precedence by ignoring the
GOOGLE_APPLICATION_CREDENTIALSenvironment variable if it is set. - Windows Compatibility: The Unix-style path
~/.config/gcloud/application_default_credentials.jsonis hardcoded, which will fail on Windows where gcloud stores ADC at%APPDATA%\gcloud\application_default_credentials.json.
We should check if GOOGLE_APPLICATION_CREDENTIALS is in the environment first, and resolve the path in a cross-platform manner.
if "GOOGLE_APPLICATION_CREDENTIALS" not in os.environ:
if os.name == "nt":
appdata = os.environ.get("APPDATA")
adc_path = os.path.join(appdata, "gcloud", "application_default_credentials.json") if appdata else ""
else:
adc_path = os.path.expanduser("~/.config/gcloud/application_default_credentials.json")
if adc_path and os.path.exists(adc_path):
try:
with open(adc_path) as f:
data = json.load(f)
if data.get("type") == "authorized_user":
payload = {
"client_id": data["client_id"],
"client_secret": data["client_secret"],
"grant_type": "refresh_token",
"refresh_token": data["refresh_token"],
}
res = requests.post("https://oauth2.googleapis.com/token", data=payload, timeout=10)
if res.ok:
id_token_val = res.json().get("id_token")
if id_token_val:
session = requests.Session()
session.headers.update({"Authorization": f"Bearer {id_token_val}"})
return session
except Exception:
pass| bucket_name, blob_name = parts[0], parts[1] | ||
|
|
||
| try: | ||
| client = storage.Client() |
There was a problem hiding this comment.
Since project_id is explicitly available as an argument to _get_outputs_from_gcs, passing project=project_id ensures the client is correctly associated with the target GCP project, which is more robust and avoids issues when the active credentials have access to multiple projects.
| client = storage.Client() | |
| client = storage.Client(project=project_id) |
| err_msg = response.json() | ||
| if isinstance(err_msg, dict): | ||
| err_msg = err_msg.get("message") or err_msg.get("error") or response.text | ||
| except ValueError: | ||
| error_text = response.text.strip() | ||
| # If the error response is HTML (e.g. standard nginx or proxy error), just extract response text preview | ||
| if error_text.startswith("<"): | ||
| err_msg = f"HTTP {response.status_code} {response.reason}" | ||
| else: | ||
| err_msg = error_text |
There was a problem hiding this comment.
If the response is not OK and response.json() is a dictionary but does not contain "message" or "error", or if response.text is empty, err_msg can end up being empty or unhelpful. Falling back to response.reason ensures that a meaningful HTTP status description (e.g., "Unauthorized" or "Bad Request") is always printed.
| err_msg = response.json() | |
| if isinstance(err_msg, dict): | |
| err_msg = err_msg.get("message") or err_msg.get("error") or response.text | |
| except ValueError: | |
| error_text = response.text.strip() | |
| # If the error response is HTML (e.g. standard nginx or proxy error), just extract response text preview | |
| if error_text.startswith("<"): | |
| err_msg = f"HTTP {response.status_code} {response.reason}" | |
| else: | |
| err_msg = error_text | |
| err_msg = response.json() | |
| if isinstance(err_msg, dict): | |
| err_msg = err_msg.get("message") or err_msg.get("error") or response.text or response.reason | |
| except ValueError: | |
| error_text = response.text.strip() | |
| # If the error response is HTML (e.g. standard nginx or proxy error), just extract response text preview | |
| if not error_text: | |
| err_msg = response.reason | |
| elif error_text.startswith("<"): | |
| err_msg = f"HTTP {response.status_code} {response.reason}" | |
| else: | |
| err_msg = error_text |
Adds custom SDMX v3 query commands
sdmx dataandsdmx availabilityunder theadmincommand group to allow easy statistical observation querying and constraint checking.Key Changes:
sdmx:sdmx availability <component_id>: Query allowed/available values for a specific dimension or attribute (e.g. sourceCountry, unit, viaOrganization) with constraints.sdmx data: Fetch statistical observations (implicitly returned in CSV format).--filteroptions into SDMX query filter queries (c[<dimension>]=<value>).--log(X-Log-SDMX header) and--multi-entity(X-Use-Multi-Entity-Schema header) flags.Usage Examples:
Querying observations from the deployed service:
(Note: Chained off 'cli-remote-state' branch/PR; we will rebase in between)