Add metadata to http provider qps and headers #33
Conversation
There was a problem hiding this comment.
As far as I can see it doesn't just add the ability, but replaces the old usage entirely, meaning it would become very hard to use the provider from a non-Studio perspective. I also don't see why this functionality needs to be part of the provider? Why can't you strip the UUID and non-enabled fields Studio-side before invoking the provider?
| @@ -171,6 +176,53 @@ async fn check_status(config: Config) -> Result<Blob> { | |||
| } | |||
|
|
|||
| async fn handle_query(config: Config, request: ProviderRequest) -> Result<Blob> { | |||
| // HACK: This KeyValueRow structure is a stop-gap measure until we | |||
There was a problem hiding this comment.
Good news, we have ArrayField support now :)
There was a problem hiding this comment.
Not in Studio yet, we need to have a design pass to see how we want the array to appear in Studio, but we're closer than ever for sure
It's hard to use this outside of Studio for now for sure. This is mostly a stop-gap measure until the UI part of
I think the main reason for that was related to data synchronization across clients: if the cell queryData doesn't contain the uuid and enabled/disabled flag, then there's no way to handle properly the synchronization of changes ("a user unchecked a query parameter") across all opened notebooks. We could add filtering on front side to re-parse querydata and prune the fields before forging the ProviderRequest, but that adds a lot of technical debt to have special ProviderCell.queryData handling when the cell intent is exactly |
Also explain why the limitation of no ';' in keys is okay for now.
|
Ah, I see. I still don’t quite see why we need the UUIDs (is it for React keys?), but that makes some sense, yes. But wouldn’t it be better to wait for full array support then, so that we can just add the metadata as optional fields? That way, if you invoke the provider from the CLI you don’t need to follow these odd formatting rules, and we don’t need to worry about semicolons in keys. |
|
We can mark array field support as a blocking issue for stable release of the http provider I think; I'm pretty sure we're really close to be able to have a working experience from Studio for this, and we didn't start the design on ArrayField from Studio or CLI perspective (do we want to always generate a UI that will store UUID and enabled/disabled before sending the data? What should the input fields look like ?) so I think waiting for ArrayField to land on front-end is going to push back the beta for too long |
|
From the CLI perspective, array fields are just like any other keys, thanks to the bracket notation. For Studio, I don't think we need to wait for the generic array UI to be implemented first, given that the HTTPS provider uses a custom UI anyway. That way the HTTPS provider doesn't need to be blocked, but at least the beta can send its data in a way that we actually plan to support going forward. |
Description
This change adds the ability to inject metadata into the http provider's extra headers and query parameters, which in turn supports the new UI for the http provider.
Note that each line in these fields is expected to be semicolon-separated, taking the form
where none of UUID, ENABLED_FLAG, or NAME should contain a semicolon. Additional semicolongs in
VALUEwill be preserved (e.g., for headers with values likeapplication/json; utf-8)UUID is something we had to add to appease React, to differentiate elements in a list.
Related PR: https://github.com/fiberplane/studio/pull/1409
Checklist
Please make sure all of these are checked before merging. Please leave items
you think are non-applicable in the list, but use strike-through (
~~) toindicate they don't apply.
- [ ] The CHANGELOG is updated.