-
Notifications
You must be signed in to change notification settings - Fork 0
feat(ingest): --continuous flag on new-datasource (buckets) #248
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
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 |
|---|---|---|
|
|
@@ -693,6 +693,11 @@ pub struct CreateArgs { | |
| #[arg(long)] | ||
| glob: Option<String>, | ||
|
|
||
| /// Keep this datasource continuously synced — refreshed incrementally on a | ||
| /// schedule, appending only newly-arrived objects (buckets only) | ||
| #[arg(long)] | ||
| continuous: bool, | ||
|
|
||
| /// Catalog type, e.g. rest (iceberg) | ||
| #[arg(long = "catalog-type")] | ||
| catalog_type: Option<String>, | ||
|
|
@@ -713,6 +718,7 @@ impl CreateArgs { | |
| || self.bucket_url.is_some() | ||
| || self.format.is_some() | ||
| || self.glob.is_some() | ||
| || self.continuous | ||
| || self.catalog_type.is_some() | ||
| || self.database_id.is_some() | ||
| } | ||
|
|
@@ -779,6 +785,7 @@ fn build_create_request( | |
| bucket_url: Some(args.bucket_url.ok_or("buckets connectors need --bucket-url")?), | ||
| file_glob: args.glob, | ||
| file_format: args.format, | ||
| continuous: args.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. super nit:
|
||
| ..Default::default() | ||
| }, | ||
| "iceberg" => IngestRequest { | ||
|
|
@@ -1837,6 +1844,7 @@ mod tests { | |
| bucket_url: None, | ||
| format: None, | ||
| glob: None, | ||
| continuous: false, | ||
| catalog_type: None, | ||
| database_id: None, | ||
| } | ||
|
|
@@ -1869,6 +1877,31 @@ mod tests { | |
| assert_eq!(req.database_id.as_deref(), Some("db_1")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn create_request_filesystem_carries_continuous_flag() { | ||
| let e = entry("buckets", "filesystem"); | ||
| let mut args = create_args(); | ||
| args.bucket_url = Some("s3://b/prefix".into()); | ||
| args.format = Some("jsonl".into()); | ||
| args.continuous = true; | ||
| let req = build_create_request(&e, args, None).unwrap(); | ||
| 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 | ||
|
|
||
| // 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!( | ||
| !serde_json::to_string(&req_off) | ||
| .unwrap() | ||
| .contains("continuous") | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn create_request_rejects_invalid_names() { | ||
| let e = entry("postgres", "sql"); | ||
|
|
||
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.
nit: the guided wizard has no matching prompt, so
continuousis unreachable interactively (not blocking).build_filesystem_interactive(src/commands/ingest.rs:527) prompts bucket URL / format / glob and leavescontinuousat itsDefaultfalse. On a terminal with no--service, that's the default path, so a user who wants continuous sync has to know to drop out of the wizard and pass the flag — andany_given()now makes--continuousalone hard-fail with "--serviceis required" rather than hinting the wizard can't do it. A one-lineConfirm/select_optional("Keep continuously synced?")in the filesystem builder would close the gap; up to you whether that belongs in this PR.