Skip to content

Fix/monitor config populate defaults#9

Open
adkinsty wants to merge 3 commits intomainfrom
fix/monitor-config-populate-defaults
Open

Fix/monitor config populate defaults#9
adkinsty wants to merge 3 commits intomainfrom
fix/monitor-config-populate-defaults

Conversation

@adkinsty
Copy link
Copy Markdown
Collaborator

fix: monitor config --enable must populate default monitor types

Problem

sodacli monitor config <dataset-id> --enable --schedule "0 6 * * *" currently fails against Soda Cloud with:

[Error] Failed validation of PublicApiUpdateDatasetRequest on the following properties:
body.metricMonitoring.datasetMetricMonitorsConfiguration:
Scan schedule and dataset metric monitors configuration must be provided
if metric monitoring is enabled

Root cause

The Cloud API now requires three fields together on metricMonitoring:

  • enabled
  • scanSchedule
  • datasetMetricMonitorsConfiguration (the list: rowCount, freshness, schema, rowsInserted, totalRowCountChange, timeliness)

dataset onboard --monitoring already sends all three via the EnableDatasetDefaults helper in internal/api/monitors.go. But monitor config --enable has its own parallel implementation in cmd/monitor.go that was only sending enabled + scanSchedule, so the Cloud API rejects it with a 400.

This silently breaks per-dataset monitor enablement in scripts that call monitor config --enable directly (including the Fizz demo bootstrap, where every subsequent monitor add then errors with Metric monitoring is not enabled on dataset).

Fix

Populate the same default monitor types from both code paths.

  • internal/api/monitors.go: export defaultDatasetMonitorTypes as DefaultDatasetMonitorTypes so it can be reused from cmd/.
  • cmd/monitor.go: when --enable is set, build a DatasetMetricMonitorCfg slice from DefaultDatasetMonitorTypes and attach it to the request, matching what EnableDatasetDefaults does.

Verification

Against a local Soda Cloud:

sodacli monitor config <dataset-id> --enable --schedule "0 6 * * *" --profile fizz-demo
# ✓ Monitor config updated for dataset '<dataset-id>'. Monitors: enabled

sodacli monitor add --dataset <dataset-id> --type column --column email --metric missing-pct --profile fizz-demo
# ✓ Column monitor created

Both succeed. Before this fix, the first command 400s and the second then errors with "Metric monitoring is not enabled on dataset".

LaurenDebruyn and others added 3 commits April 10, 2026 11:14
Add --scheme flag to auth login so users can connect over HTTP for local
dev. Centralize host+scheme defaulting in Profile.BaseURL() and use it
everywhere (API client, auth status, login confirmation). Fix response
body leak in UpdateMetricMonitoring by decoding the POST response.
Remove redundant UpdateMetricMonitoringRequest struct in favor of
MetricMonitoringSettings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of a separate --scheme parameter, users can now specify
http:// or https:// as part of the --host value. Defaults updated
to https://cloud.soda.io to make the convention visible.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Soda Cloud's PublicApiUpdateDatasetRequest validator now rejects requests
with metricMonitoring.enabled=true unless datasetMetricMonitorsConfiguration
is also provided:

  body.metricMonitoring.datasetMetricMonitorsConfiguration:
  Scan schedule and dataset metric monitors configuration must be provided
  if metric monitoring is enabled

`dataset onboard --monitoring` already sends the list (via EnableDatasetDefaults),
but `monitor config --enable --schedule ...` was only sending Enabled + ScanSchedule.
Populate the same default monitor types (rowCount, freshness, schema,
rowsInserted, totalRowCountChange, timeliness) so both code paths stay in sync.

- Export defaultDatasetMonitorTypes as DefaultDatasetMonitorTypes for reuse
  across cmd/ and internal/api/.
- Populate req.DatasetMetricMonitorsConfiguration when --enable is set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants