Extend capabilities of the hidden, capture interval field
#1319
Conversation
kiahna-tucker
changed the title
interval field to capture workflows.interval field to capture workflows
| return isFinite(numericSegment) ? numericSegment : -1; | ||
| }); | ||
|
|
||
| return { h: hours, m: minutes, s: seconds }; |
There was a problem hiding this comment.
What is driving the renaming to a single letter? The only consumer of this just renames them back to what they were.
There was a problem hiding this comment.
The value of the select menu items and the need for getCaptureIntervalSegment, which is the second consumer of these functions.
There was a problem hiding this comment.
Please add a new more "insane edgecases". Since today you can parse 999999999999999999999999999999999999999s today and that should probably be called out.
There was a problem hiding this comment.
There are limits the to values that can be used to test but the test coverage can be extended within reason.
| const strippedInterval = interval.endsWith('i') | ||
| ? interval.slice(0, interval.length - 1) | ||
| : interval; |
There was a problem hiding this comment.
If we are doing stuff like this then the i needs to be a const and this kind of action should go into the utils.
|
So the coding is fine - however we have way too many edge cases to try to work around with now. We'll need to complete estuary/flow#1750 before the UI can handle durations. |
kiahna-tucker
changed the title
interval field to capture workflowsinterval field
Issues
The issues directly below are advanced by this PR:
#1194
Changes
1194
The following features are included in this PR:
intervalproperty in capture workflows.NOTE: The capture interval field will remain hidden as an investigation into the
Durationformats supported by the backend is underway. Additionally, it should be noted thatluxonshould be leveraged to handle converting to and from strings in aDurationformat (ref:luxondocumentation).Tests
Manually tested
Approaches to testing are as follows:
Automated tests
Test coverage added for the following utils:
formatCaptureIntervalgetCaptureIntervalSegmentPlaywright tests ran locally
Screenshots
N/A