Page MenuHomePhabricator

bengotow (Ben Gotow)
User

Projects

User does not belong to any projects.

User Details

User Since
May 9 2019, 3:45 PM (95 w, 20 h)

Recent Activity

Yesterday

bengotow accepted D6792: [dagit] Separate pipeline and snapshot links in Run header.
Thu, Mar 4, 8:19 PM
bengotow accepted D6791: [dagit] Allow Group wrapping.
Thu, Mar 4, 8:18 PM
bengotow accepted D6744: [dagit] Move mode alongside pipeline name.

This seems like a nice presentation - I think we've been struggling for a while to find a consistent home for "mode" that doesn't make it look like a tag. I like having the <PipelineAndMode> component to wrap up the UI - I wonder if we could push the href generation down into that component in any way? I'm not sure how easy it'd be to retrieve repoAddress within the component...

Thu, Mar 4, 8:17 PM
bengotow accepted D6724: [dagit] Localize elapsed time and logs timestamps.

Oh this is super cool - nice that we don't need moment for this sort of thing these days.

Thu, Mar 4, 8:14 PM

Mon, Mar 1

bengotow accepted D6704: [dagit] Add global search to current left nav.

This looks great to me! Also agree with getting rid of the search bars in each section 👍

Mon, Mar 1, 4:16 PM
bengotow accepted D6711: [dagit] Some dependency upgrades.
Mon, Mar 1, 4:14 PM

Tue, Feb 23

bengotow closed D6490: Force scrollbar visibility on partition matrix to make behavior more clear #3638.
Tue, Feb 23, 10:39 PM
bengotow committed R1:cdf946aaaf10: Force scrollbar visibility on partition matrix to make behavior more clear #3638 (authored by bengotow).
Force scrollbar visibility on partition matrix to make behavior more clear #3638
Tue, Feb 23, 10:39 PM
bengotow closed D6491: In backfill modal, explain ”Re-execute From Failure”, remove “Re-execute From Last Run”.
Tue, Feb 23, 10:38 PM
bengotow committed R1:e32c9ce982d0: In backfill modal, explain ”Re-execute From Failure”, remove “Re-execute From… (authored by bengotow).
In backfill modal, explain ”Re-execute From Failure”, remove “Re-execute From…
Tue, Feb 23, 10:38 PM
bengotow accepted D6547: [dagit] Simplify logs filtering input.

This looks awesome! Thanks for writing tests too since I imagine this will get pulled into the other places we have a tokenizing text field.

Tue, Feb 23, 4:08 PM

Mon, Feb 22

bengotow accepted D6572: [dagit] Use Python event types in log filter view.

This looks good to me, I don't think the capitalized strings in the logs are too scary...

Mon, Feb 22, 9:42 PM
bengotow accepted D6589: [dagit] Add repo location reload to object headers.

This looks great! Nice we already had useRepositoryLocationReload, I forgot it was a hook!

Mon, Feb 22, 9:06 PM
bengotow accepted D6628: [dagit] Add more useful things to Pipelines list.

This looks great to me! It does seem it'd be nice to have the fuse implementation pulled out into a helper hook because it gets a little technical, but there may not be anywhere else that we could re-use it so it's probably not worth factoring apart into two pieces.

Mon, Feb 22, 8:02 PM
bengotow accepted D6606: [dagit] Fix Asset search selection.

This looks good to me!

Mon, Feb 22, 7:49 PM
bengotow accepted D6564: [dagit] Flip hideNonMatches default.
Mon, Feb 22, 7:48 PM

Mon, Feb 15

bengotow requested review of D6491: In backfill modal, explain ”Re-execute From Failure”, remove “Re-execute From Last Run”.
Mon, Feb 15, 5:58 PM
bengotow requested review of D6490: Force scrollbar visibility on partition matrix to make behavior more clear #3638.
Mon, Feb 15, 5:25 PM
bengotow accepted D6403: [dagit] RFC: Instance status dot in left nav.

I'll let other folks weigh in but the JS for this looks great. I think that you're right - the dot should either display detailed info when you hover over it (as opposed to Warnings found) or they should be in a consolidated spot on the instance tab (or maybe each tab of the instance UI gets it's own yellow dot so you can click into the right tab? Probably overkill...)

Mon, Feb 15, 5:01 PM
bengotow accepted D6441: [dagit] Shortcut handlers for simple left nav.

This looks great! I like the new shortcut annotation styling too, matches the new sidebar a bit better 👍

Mon, Feb 15, 5:01 PM
bengotow accepted D6447: [dagit] Some loading-state cleanup.
Mon, Feb 15, 5:00 PM

Thu, Feb 11

bengotow accepted D6413: [dagit] Adjust run status colors.

Oh this is great, I really like the slow pulsing of the queued runs. I feel like it /could/ be a little distracting if different runs on the page pulse out of sync, but I imagine there aren't many places where we'd run in to that. Let's ship it!

Thu, Feb 11, 2:13 AM
bengotow accepted D6405: [dagit] Font smoothing and default color.

Oh wow this is super noticeable. I didn't think to try this, looks much nicer!

Thu, Feb 11, 2:11 AM
bengotow accepted D6402: [dagit] Consistent blue color for nav, search.
Thu, Feb 11, 2:10 AM
bengotow accepted D6386: [dagit] Tweak some spacing following header change.
Thu, Feb 11, 2:10 AM

Tue, Feb 9

bengotow accepted D6367: [dagit] Consistent top nav.

This looks great! I like this direction and the consistency of the header + subtitle approach a lot. Just have a couple drive-by comments below, but I think we should merge this ASAP.

Tue, Feb 9, 6:08 PM

Fri, Feb 5

bengotow accepted D6333: [dagit] Fix Gantt chart scale.
Fri, Feb 5, 8:50 PM
bengotow added a comment to D6333: [dagit] Fix Gantt chart scale.

Yep I think this will do it 👍 I think this must have become a problem when we started emitting logs during the "queued" phase and putting a big gap between firstLogAt and startedPipelineAt.

Fri, Feb 5, 8:50 PM
bengotow accepted D6324: [dagit] QueryCountdown to consolidate some repetitive code.

This looks like a great refactor to me! Big fan of making these consistent.

Fri, Feb 5, 4:41 PM
bengotow accepted D6323: Repair LeftNavSimple.stories.
Fri, Feb 5, 4:36 PM
bengotow accepted D6322: Repair pointer-events with Group margins.

LGTM!

Fri, Feb 5, 4:36 PM

Feb 2 2021

bengotow closed D6226: Update the headers on the run table to call out Run ID vs Snapshot ID, pipeline more clearly.
Feb 2 2021, 10:57 PM
bengotow committed R1:15008477a463: Update the headers on the run table to call out Run ID vs Snapshot ID, pipeline… (authored by bengotow).
Update the headers on the run table to call out Run ID vs Snapshot ID, pipeline…
Feb 2 2021, 10:57 PM
bengotow closed D6232: Use a finite spinner for the partition view loading indicator.
Feb 2 2021, 10:57 PM
bengotow committed R1:4493f47fbca9: Use a finite spinner for the partition view loading indicator (authored by bengotow).
Use a finite spinner for the partition view loading indicator
Feb 2 2021, 10:57 PM
bengotow accepted D6256: [dagit] Set per-snapshot run filters as "permanent".

Ah I like this styling above the input, good call!

Feb 2 2021, 9:24 PM
bengotow accepted D6252: [dagit] Add flag to enable new left nav.

Ahh this is exciting! Look forward to trying it in the app - the code looks great to me 👍

Feb 2 2021, 9:23 PM

Feb 1 2021

bengotow accepted D6236: [dagit] Left Nav exploration.

I think we talked about this Thursday - I'm a big fan of incrementally introducing the repository concept and having this flat list of pipelines. The components look good to me!

Feb 1 2021, 10:08 PM
bengotow added a comment to D6224: toy pipeline that generates a partitioned asset.

Nice! I think that longitudinal_pipeline also has a partitioned costs_dashboard asset and I've been using that one to test the asset materialization views but it'll be nice to have one that emits metadata entries.

Feb 1 2021, 10:05 PM
bengotow accepted D6230: [dagit] Time components.

Looks good to me! I'm surprised timestampToString is used outside the time folder since it's not user-settings-aware, but we can audit that later!

Feb 1 2021, 10:03 PM
bengotow accepted D6231: [dagit] Roots for Pipelines, Settings.

Looks good to me!

Feb 1 2021, 10:01 PM
bengotow closed D6228: [dagit] Fix responsiveness of partition graphs, flex:1 without min-width prevents shrinking.
Feb 1 2021, 9:54 PM
bengotow committed R1:c636518ddbba: [dagit] Fix responsiveness of partition graphs, flex:1 without min-width… (authored by bengotow).
[dagit] Fix responsiveness of partition graphs, flex:1 without min-width…
Feb 1 2021, 9:54 PM
bengotow closed D6227: Hide partial failure % from the run matrix unless you “Show Previous”.
Feb 1 2021, 9:54 PM
bengotow committed R1:b59932b416eb: Hide partial failure % from the run matrix unless you “Show Previous” (authored by bengotow).
Hide partial failure % from the run matrix unless you “Show Previous”
Feb 1 2021, 9:54 PM
bengotow requested review of D6232: Use a finite spinner for the partition view loading indicator.
Feb 1 2021, 8:57 PM
bengotow requested review of D6228: [dagit] Fix responsiveness of partition graphs, flex:1 without min-width prevents shrinking.
Feb 1 2021, 6:18 PM
bengotow requested review of D6227: Hide partial failure % from the run matrix unless you “Show Previous”.
Feb 1 2021, 6:04 PM
bengotow requested review of D6226: Update the headers on the run table to call out Run ID vs Snapshot ID, pipeline more clearly.
Feb 1 2021, 5:31 PM
bengotow accepted D6152: [dagit] Prompt to refresh config when repo is refreshed.

This looks great and I like the banner styling a lot. I guess there's always a React render after the refresh function is run, so invalidateConfigsForRepo doesn't need to trigger a render after it modifies local storage? It might be good to add a small note to that function to note that the caller should trigger a page refresh in some way, because interacting with the config editor after calling invalidateConfigsForRepo would overwrite the local storage state back to match the config editor's state (I think?) A cheap alternative might be to fire a custom event on the document that components could listen to and handle a repo reload, and then set the dirty bit from the ExecutionSessionContainer, but I think this is clear and is less code!

Feb 1 2021, 4:23 PM
bengotow accepted D6169: [dagit] Clean up excess spinners on Runs page.

Looks like a safe bet to me! There are so many spinners on here.

Feb 1 2021, 4:15 PM
bengotow accepted D6191: [dagit] Use Spinner for Loading component.
Feb 1 2021, 4:04 PM
bengotow accepted D6188: [dagit] Standardize and slow down Spinner.

This looks good to me! Big fan of standardizing these, even if it makes things less pixel-perfect in a button or two. 🙌

Feb 1 2021, 4:04 PM
bengotow accepted D6181: [dagit] RFC: Launch runs in the same tab.

Hmm this was definitely intentional but I don't quite remember the context around it. I went back and found the original diff where that was added (https://github.com/dagster-io/dagster/commit/02b1d07b6b00407c9533d4d718efc6195620961a#diff-0788ce9e6243e61675f971ee6022361b57a9d1a0b32fcff11036acf8583e22fb) but there's not much of an explanation and it's almost two years old. I'd say we merge this and give it a spin, and if there's interest in opening things in separate windows we could potentially allow cmd-clicking on the buttons, etc to send openInNewWindow: true.

Feb 1 2021, 3:53 PM

Jan 31 2021

bengotow closed D6132: UI support for dynamic outputs, Run view state cleanup.
Jan 31 2021, 5:45 PM
bengotow committed R1:5f1d42423a3d: UI support for dynamic outputs, Run view state cleanup (authored by bengotow).
UI support for dynamic outputs, Run view state cleanup
Jan 31 2021, 5:44 PM
bengotow updated the diff for D6132: UI support for dynamic outputs, Run view state cleanup.

Patch tests

Jan 31 2021, 5:27 PM
bengotow updated the diff for D6132: UI support for dynamic outputs, Run view state cleanup.

Rebase and address diff feedback

Jan 31 2021, 5:17 PM
bengotow closed D6029: Flip asset materialization UI so newest time/partition is on the left.
Jan 31 2021, 4:51 PM
bengotow committed R1:df93cfde6c4e: Flip asset materialization UI so newest time/partition is on the left (authored by bengotow).
Flip asset materialization UI so newest time/partition is on the left
Jan 31 2021, 4:51 PM

Jan 26 2021

bengotow planned changes to D6132: UI support for dynamic outputs, Run view state cleanup.
Jan 26 2021, 7:38 PM

Jan 25 2021

bengotow accepted D6147: [dagit] Fix scrolling in GanttStatusPanel.
Jan 25 2021, 7:26 PM
bengotow accepted D6117: [dagit] Simplify Table styles.

I think this is a positive change - removing the alternating background seems fine and makes the rows appear less clickable (which I think is a *good* thing because our rows are largely not clickable.)

Jan 25 2021, 7:25 PM
bengotow accepted D6144: [dagit] Repair launch button loading state.

Oh good catch—seems like a major oversight that this wasn't awaiting the mutation!

Jan 25 2021, 7:23 PM
bengotow accepted D6080: Set tick history graph tooltip content based on click/hover events.

This looks great to me!

Jan 25 2021, 6:01 PM

Jan 23 2021

bengotow requested review of D6132: UI support for dynamic outputs, Run view state cleanup.
Jan 23 2021, 11:50 PM
bengotow updated the diff for D6029: Flip asset materialization UI so newest time/partition is on the left.
  • Instead of flipping, scroll the asset materialization grid to the right by default
  • Fix React warnings in the useViewport component, some useCallback's missing
Jan 23 2021, 10:34 PM

Jan 19 2021

bengotow accepted D6049: [dagit] Eliminate static fragments property.

Ahh this is great! +1 on this being a blocker on switching to functional components without weird Typescript hacks. I don't think I've run into the circular dependency issue, but I imagine that's a cryptic rabbit hole we should avoid!

Jan 19 2021, 10:59 PM

Jan 15 2021

bengotow added reviewers for D6029: Flip asset materialization UI so newest time/partition is on the left: schrockn, dish.
Jan 15 2021, 4:25 PM
bengotow updated the summary of D6029: Flip asset materialization UI so newest time/partition is on the left.
Jan 15 2021, 4:25 PM
bengotow requested review of D6029: Flip asset materialization UI so newest time/partition is on the left.
Jan 15 2021, 4:07 PM

Jan 8 2021

bengotow accepted D5870: Upgrade TypeScript, react-scripts.
Jan 8 2021, 4:35 AM
bengotow accepted D5868: formatElapsedTime proposal for <10s.
Jan 8 2021, 4:35 AM
bengotow accepted D5856: [dagit] Make sections of Instance config page linkable.

Wow this is a cool idea, code looks good to me! I like the highlighted styling too, nice UI bonus 🙌

Jan 8 2021, 4:35 AM
bengotow accepted D5839: [dagit] Add `id` to PartitionSet.
Jan 8 2021, 4:32 AM

Jan 6 2021

bengotow accepted D5837: [dagit] Select run rows with Shift+click.

This is great! I think this is a really nice use of useReducer and I'm surprised it ended up being so concise, when you described this yesterday I imagined it being about 50% more code than this :-p Will be a nice feature to have!

Jan 6 2021, 4:42 PM
bengotow accepted D5822: [dagit] Consolidate pieces into "Instance details".

This looks great to me! I'm a big fan of this change because I think it'll more clearly communicate the repo vs. instance distinction between the two UIs that show sensors and schedules 👍

Jan 6 2021, 4:38 PM
bengotow accepted D5807: [dagit] Information header on RunRoot.
Jan 6 2021, 4:33 PM
bengotow closed D5723: More consistent modals for “Show JSON”, “Show MD”, whitespace in “Copy”.
Jan 6 2021, 4:19 PM
bengotow committed R1:ff80852657dd: More consistent modals for “Show JSON”, “Show MD”, whitespace in “Copy” (authored by bengotow).
More consistent modals for “Show JSON”, “Show MD”, whitespace in “Copy”
Jan 6 2021, 4:19 PM
bengotow closed D5722: Update Asset Detail header to reflect new Schedule header design.
Jan 6 2021, 4:19 PM
bengotow committed R1:2e03537cc275: Update Asset Detail header to reflect new Schedule header design (authored by bengotow).
Update Asset Detail header to reflect new Schedule header design
Jan 6 2021, 4:19 PM
bengotow updated the diff for D5722: Update Asset Detail header to reflect new Schedule header design.

Update asset header to use Group

Jan 6 2021, 4:09 PM
bengotow planned changes to D5722: Update Asset Detail header to reflect new Schedule header design.
Jan 6 2021, 4:05 PM

Jan 4 2021

bengotow accepted D5786: [dagit] Allow termination of entire backfill.

This looks great to me!

Jan 4 2021, 5:45 PM

Dec 23 2020

bengotow accepted D5716: [dagit] TimeElapsed cleanup.

This looks great to me! Cool use of tabular-nums in here too, I forgot that was a thing!

Dec 23 2020, 9:34 PM
bengotow resigned from D5762: specify partition for asset events.

Will defer to @sandyryza for Python review but this looks good! Excited to get this in!

Dec 23 2020, 9:31 PM

Dec 21 2020

bengotow requested review of D5723: More consistent modals for “Show JSON”, “Show MD”, whitespace in “Copy”.
Dec 21 2020, 11:04 PM
bengotow requested review of D5722: Update Asset Detail header to reflect new Schedule header design.
Dec 21 2020, 10:58 PM
bengotow abandoned D5637: Add an HTML metadata entry type, allow JSON and HTML to be inlined into logs.

Sounds good! Yeah I agree doing this on hosted dagit would be bad. There are a few improvements in this diff separate from the actual HTML / JSON / inlining feature and I'll pull those into a separate tiny diff here in a bit.

Dec 21 2020, 4:55 PM
bengotow accepted D5713: [dagit] Give TopNav some z precedence.
Dec 21 2020, 4:53 PM

Dec 17 2020

bengotow closed D5630: Summary: Misc small Dagit fixes - #3343, #2568, #1015.
Dec 17 2020, 3:51 AM
bengotow committed R1:644d62024e00: Summary: Misc small Dagit fixes - #3343, #2568, #1015 (authored by bengotow).
Summary: Misc small Dagit fixes - #3343, #2568, #1015
Dec 17 2020, 3:51 AM
bengotow closed D5638: Remove materializations / expectations state tracking from RunMetadataProvider.
Dec 17 2020, 3:51 AM
bengotow committed R1:26df5457345f: Remove materializations / expectations state tracking from RunMetadataProvider (authored by bengotow).
Remove materializations / expectations state tracking from RunMetadataProvider
Dec 17 2020, 3:51 AM

Dec 16 2020

bengotow requested review of D5637: Add an HTML metadata entry type, allow JSON and HTML to be inlined into logs.
Dec 16 2020, 7:57 PM
bengotow requested review of D5638: Remove materializations / expectations state tracking from RunMetadataProvider.
Dec 16 2020, 6:59 AM
bengotow accepted D5620: [dagit] Pause Gaant when websocket disconnects.
Dec 16 2020, 5:31 AM
bengotow accepted D5631: [dagit] Gaant -> Gantt.

Oh hey had no idea, LGTM!

Dec 16 2020, 5:31 AM

Dec 15 2020

bengotow requested review of D5630: Summary: Misc small Dagit fixes - #3343, #2568, #1015.
Dec 15 2020, 10:29 PM