Use new API to reference programs and connectors#1157
Use new API to reference programs and connectors#1157Karakatiza666 merged 1 commit intonames-as-refsfrom
Conversation
82f9128 to
f793b53
Compare
31987e6 to
098bd74
Compare
|
🤖 Meticulous replayed 14 user sessions and took 6 visual snapshots. Meticulous did not run on names-as-refs of the names-as-refs branch and so there was nothing to compare against. Please merge your pull request for setting up Meticulous in CI and ensure that it’s running on push events to the names-as-refs branch. Last updated for commit 5340f99. This comment will update as new commits are pushed. |
098bd74 to
0569626
Compare
lalithsuresh
left a comment
There was a problem hiding this comment.
LGTM. Let's merge tomorrow after the WebConsole test framework is ready.
| // could cancel potentially in-progress saves (started by client action). | ||
|
|
||
| const project = projectsQuery.data.find(p => p.program_id === descriptor.program_id) | ||
| const project = projectsQuery.data.find(p => p.name === descriptor.program_name) |
There was a problem hiding this comment.
why not call both of them "program_name"?
Then you don't have to think
| // programCode: (programName: string) => ProgramsService.getProgram(programName, true), // TODO: Uncomment with an API update | ||
| programCode: (programName: string) => | ||
| ProgramsService.getPrograms(undefined, programName, true).then(p => | ||
| p.length ? p[0] : Promise.reject(new Error('/v0/programs: ' + programName + ' not found')) |
There was a problem hiding this comment.
Why the /v0? Who consumes this error?
There was a problem hiding this comment.
In a previous implementation when programCode with a given programId was not found (should be super rare) an error was not handled, so it would bubble up (and shown in a bottom left red error popup, i think). Here I am manually throwing an error to keep this behavior.
There was a problem hiding this comment.
The error message is not 1 to 1 to previous one, just did a simple reasonable one
There was a problem hiding this comment.
But why the "/v0/programs" is part of the error?
There was a problem hiding this comment.
This "v0" is an internal thing that no one needs to know about. I don't think it should be part of the error message
There was a problem hiding this comment.
/v0/programs is the REST API endpoint. I agree with Mihai that we probably don't need "/v0/programs" in the error message string. It also feels redundant with the error the API server will give back anyway when accessing programs that don't exist.
There was a problem hiding this comment.
@lalithsuresh will api return 4XX if /v0/programs?name=... doesn't find a program? I assumed it would return empty list.
There was a problem hiding this comment.
Upd: It does return 4xx. Patched the custom error out
…pipelines Signed-off-by: George <bulakh.96@gmail.com>
0569626 to
5340f99
Compare
…eConfig DevTweaks Option<f64> fields (bloom_false_positive_rate, balancer_balance_tax, etc.) fail to deserialize when nested inside PipelineConfig, which uses #[serde(flatten)] on RuntimeConfig. This is a known serde_json bug (#1157) triggered by the workspace-wide arbitrary_precision feature: the Content buffer represents numbers as maps, producing "invalid type: map, expected f64". Apply the existing serde_via_value::deserialize workaround (already used by ResourceConfig.cpu_cores_min/max) to all four Option<f64> fields in DevTweaks. Also add the chrono "alloc" feature to fix a pre-existing compile error in adapter_stats.rs (to_rfc3339_opts returns String, which requires alloc). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Change UI to reference programs and connectors by name when managing pipelines