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

Expose resource pointers and resource config update through flow-web crate #1863

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

travjenkins
Copy link
Member

@travjenkins travjenkins commented Jan 10, 2025

Description:

#1760

Workflow steps:

(How does one use this feature, and how has it changed)

Documentation links affected:

(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)

Notes for reviewers:

Related UI task


This change is Reviewable

Comment on lines +210 to +212
// Outputting as a string as I just could NOT get it to return JSON correctly
let output = serde_json::to_string(&resource_spec).unwrap();
serde_wasm_bindgen::to_value(&{ output }).map_err(|err| JsValue::from_str(&format!("{err:?}")))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I quit trying to get this to return a JSON object to JS and just returned a string and parse it in the JS.

I kept getting the Value back (map of key values) and got frustrated and stopped trying.

wasmResponse

@@ -233,6 +233,42 @@ fn test_end_to_end_extend_read_bundle() {
);
}

#[wasm_bindgen_test]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure does not cover everything and is really more of a happy path test. Did not feel like writing every permutation with this or that property missing was necessary right now.

@travjenkins travjenkins changed the title Travjenkins/move functions from agent for wasm Expose resource pointers and resource config update through flow-web crate Jan 23, 2025
travjenkins and others added 3 commits January 23, 2025 16:22
Making this a default feature causes it to be enabled when building the
`flow-web` crate, even with `default-features = false`.  So removing combine
from default features and making other dependents explicitly enable it, to try
to get flow-web building.
@travjenkins travjenkins added the change:planned This is a planned change label Jan 30, 2025
@travjenkins travjenkins marked this pull request as ready for review January 30, 2025 23:45
@travjenkins travjenkins requested review from mdibaiee, psFried and a team January 30, 2025 23:45
Fixes a regression in how we generate resource configs when adding bindings to
a materialization.  This regressed as part of introducing the ability to
specify the target schema as part of the `sourceCapture` configuration.  When
adding a binding to a materialization that uses `targetSchema: fromSourceName`,
if the resource spec schema includes an `x-schema-name` annotation, then we'll
only use the last path component of the collection name to set
`x-collection-name`.  But if the materialization uses `targetSchema:
leaveEmpty`, or if the resource spec schema does not include an `x-schema-name`
annotation, then we'll use the concatenation of the last two path components of
the collection name, just like we used to.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:planned This is a planned change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants