-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ingest): continuous datasources are not validate_only #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -828,8 +828,14 @@ fn build_create_request( | |
| } | ||
| } | ||
| }; | ||
| // Adding a datasource discovers the schema only — never loads data. | ||
| req.validate_only = true; | ||
| // Adding a datasource discovers the schema only — never loads data — EXCEPT | ||
| // a continuous one, which is a persistent, self-loading datasource the | ||
| // scheduler keeps synced. Sending validate_only with continuous is | ||
| // contradictory (a one-off preview that is also permanently synced): the | ||
| // worker rejects the pair 422, and before it did, the datasource was | ||
| // re-run every tick but fell to the full-replace path — reloading the whole | ||
| // bucket forever. So continuous datasources are created ready to sync. | ||
| req.validate_only = !req.continuous; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: the post-create UX still assumes a validate-only add (not blocking). With Threading the |
||
| req.name = args.name; | ||
| req.database_id = args.database_id; | ||
| Ok(req) | ||
|
|
@@ -1896,13 +1902,17 @@ mod tests { | |
| assert_eq!(req.family, "filesystem"); | ||
| assert_eq!(req.bucket_url.as_deref(), Some("s3://b/prefix")); | ||
| assert!(req.continuous); // --continuous rides through to the request body | ||
| // A continuous datasource is self-loading, so it is NOT validate_only — | ||
| // the worker 422s the pair, and it was the original full-replace bug. | ||
| assert!(!req.validate_only); | ||
|
|
||
| // Default is off, and it serializes only when true (skip_serializing_if). | ||
| let mut off = create_args(); | ||
| off.bucket_url = Some("s3://b".into()); | ||
| off.format = Some("jsonl".into()); | ||
| let req_off = build_create_request(&e, off, None).unwrap(); | ||
| assert!(!req_off.continuous); | ||
| assert!(req_off.validate_only); // a non-continuous add still discovers schema only | ||
| assert!( | ||
| !serde_json::to_string(&req_off) | ||
| .unwrap() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit:
skills/hotdata/SKILL.md:250still saysnew-datasource"Validates credentials + discovers the schema; loads NO data", and the--continuousbullet at line 259 only describes the scheduled re-runs. After this change the--continuousadd does load data up front. Worth a clause on line 259 noting the create seeds the initial load (not blocking).