Conversation
051e101 to
e2c00de
Compare
Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
…nector icon Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
e2c00de to
1ac5ab9
Compare
|
@Karakatiza666 that screen recording looks wrong. Why is the progress bar moving forward, while the number of committed records goes from 27k/30K back to 17k/30k? |
mythical-fred
left a comment
There was a problem hiding this comment.
Two soft issues inline. pipelineMetrics.ts has non-trivial logic (io_active detection, transaction_phase lowercasing, aggregate reductions) that would be well-served by Vitest unit tests — worth chipping away at (npm install -D vitest @testing-library/svelte jsdom).
| io_active: | ||
| prev !== undefined && cur.metrics.total_records > prev.metrics.total_records, | ||
| transaction_phase: connectorPhase | ||
| ? (connectorPhase.toLowerCase() as 'started' | 'committed') |
There was a problem hiding this comment.
Unsafe assertion: if the API returns an unexpected casing or a future phase value, this silently produces a wrong type. Safer:
const p = connectorPhase.toLowerCase()
transaction_phase: (p === 'started' || p === 'committed') ? p : undefined| global: { | ||
| transaction_status: 'NoTransaction', | ||
| transaction_initiators: { initiated_by_connectors: {} } | ||
| } as GlobalMetricsTimestamp |
There was a problem hiding this comment.
The as GlobalMetricsTimestamp cast hides incomplete initialization — only transaction_status and transaction_initiators are present. If any consumer reads e.g. global.start_time before the first real poll, it silently gets undefined. Consider typing the empty value explicitly (e.g., Partial<GlobalMetricsTimestamp>) and making consumers handle it, or provide a complete zero value.
|
@lalithsuresh the number of records processed + total is actually only for operators currently being committed, not across the whole transaction. |
|
@Karakatiza666 what is the progress bar based on? |
|
Three values: |
|
@ryzhyk should know the most useful info here to showcase. |
|
@blp please take a look, too |
|
Nit
Question:
|
|
The screenshots are confusing because I changed the order midway.
Maximum two icons are shown at a time, they are prioritized based on importance to the current state. This helps visualize the most important information about the connector in any state without overloading the UI with a lot of icons.
I will ask our designer for a better icon; this one made sense to me. |
Changes:
Time elapsed in the transaction is not yet available in API.
Manual testing: tested using a custom simulator that cycles through pipeline transaction states, and by observing a pipeline with deltalake inputs that is configured with transaction_mode: snapshot
Screencast.from.2026-03-04.16-07-11.webm