Skip to content
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

Phil/discover name validation #1365

Merged
merged 3 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/agent-sql/src/drafts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub async fn create(
Ok(row.id)
}

#[tracing::instrument(err, skip(spec, txn))]
pub async fn upsert_spec<S>(
draft_id: Id,
catalog_name: &str,
Expand Down Expand Up @@ -53,7 +54,6 @@ where
)
.fetch_one(&mut *txn)
.await?;

Ok(())
}

Expand Down
19 changes: 19 additions & 0 deletions crates/agent/src/discovers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,25 @@ impl DiscoverHandler {
specs::parse_response(endpoint_config, image_name, image_tag, discover_output)
.context("converting discovery response into specs")?;

if discovered_bindings
.iter()
.any(|b| b.recommended_name.is_empty())
{
tracing::error!(
?discovered_bindings,
%capture_name,
%draft_id,
%image_name,
%image_tag,
"connector discovered response includes a binding with an empty recommended_name"
);
return Ok(Err(vec![draft::Error {
catalog_name: capture_name.to_string(),
scope: None,
detail: "connector protocol error: a binding was missing 'recommended_name'. Please contact support for assistance".to_string(),
}]));
}

// Catalog we'll build up with the merged capture and collections.
let mut catalog = models::Catalog::default();

Expand Down
10 changes: 6 additions & 4 deletions crates/agent/src/discovers/specs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,10 @@ pub fn merge_collections(
}

fn normalize_recommended_name(name: &str) -> String {
let parts: Vec<_> = models::Collection::regex()
use itertools::Itertools;
let mut parts = models::Collection::regex()
.find_iter(name)
.map(|m| m.as_str())
.collect();
.map(|m| models::collate::normalize(m.as_str().chars()).collect::<String>());

parts.join("_")
}
Expand Down Expand Up @@ -681,8 +681,10 @@ mod tests {
for (name, expect) in [
("Foo", "Foo"),
("foo/bar", "foo/bar"),
("Faſt/Carſ", "Fast/Cars"), // First form is denormalized, assert that it gets NFKC normalized
("/", ""), // just documenting a weird edge case
("/foo/bar//baz/", "foo/bar_baz"), // Invalid leading, middle, & trailing slash.
("#੫൬ , bar-_!", "੫൬_bar-_"), // Invalid leading, middle, & trailing chars.
("#੫൬ , bar-_!", "੫൬_bar-_"), // Invalid leading, middle, & trailing chars.
("One! two/_three", "One_two/_three"),
] {
assert_eq!(
Expand Down
Loading