User Details
- User Since
- May 9 2019, 3:45 PM (212 w, 1 d)
- Roles
- Disabled
Aug 4 2021
Thanks for following up on this one! Does this mean that unlinked inputs would generally make sense to display in an accent color now? (re: https://github.com/dagster-io/dagster/issues/4407)
This LGTM! Excited to give this a spin
The JS changes here look great. I'd +1 showing the partition set name all the time with a link, and then just offering to "Show Coverage" or something like that to reveal "Missing", etc.
Aug 2 2021
Jul 29 2021
Jul 27 2021
This looks good to me! Added a couple inline comments but I think this is a nice improvement.
This looks good! I could have sworn there was more of a reason these were class components (shouldComponentUpdate or something like that...) but it doesn't seem like it. The new SFC implementations look equivalent to me sans that one setNodes([]) call 👍
This looks good to me! Just added an inline comment about the python syntax but feel free to ignore, you've thought about this 100x more and I'm just getting up to speed with the user stories around this!
Oh wow haven't seen this before, good find
Jul 16 2021
This looks good to me! +1 for merging and kicking the tires on dev.
Jul 8 2021
Ahh good catch! I think I had too many tabs open to notice this one 🙈
Jul 6 2021
- Fix tests
- Revert snapshot change - only broken for me?
- diff feedback
This looks great! Thanks for addressing the other stuff, I just added one comment about the presets.length == 1 && partitionSets case but I think this is good to go!
Code lgtm, but I agree we could play with the UI treatment a bit. Is the debug download feature just for folks to send to us, or can they inspect it themselves? If it's something we want people to find easily, we could potentially put two small icon buttons side by side in the top right area with the "tags" icon and the "download" icon and show the labels on hover? I think the tags icon is good enough people will guess what it does and we might end up with a whole toolbar of actions up there eventually...
Jul 1 2021
This looks good! It looks like runConfigYaml isn't used down in the ConfigEditorConfigPicker but otherwise this seems like a great change
Rebase
Jun 30 2021
I think it'd be nice to go ahead and rip out /flags or have it redirect to this new page, seems like having both isn't super useful.
This looks good to me. I think from a user perspective it'd almost be better to fallback quietly to window.alert(stuffForClipboard) so they can select+copy out of there rather than hiding the buttons. I worry people will think the button was removed and file a ticket complaining about it. Didn't realize this was conditionally unavailable though! Super good to know 👍
This looks great! Amazing how much better useQuery is than the old stuff...
Jun 18 2021
Jun 16 2021
Rebase
Ahh this looks great! Want to go ahead and merge this and i'll update the icons diff?
Jun 15 2021
- Fix title so composite solids look like graphs when in explorer is shown from /graphs/
- Fix a few stray references to Pipelines in the workspcae repo explorer
- Block access to the definition at the old paths, redirect to the overview (making overview the default)
- Address diff feedback
Rebase
- Address diff feedback
- Block access to the definition at the old paths, redirect to the overview (making overview the default)
- Fix title so composite solids look like graphs when in explorer is shown from /graphs/
- Fix a few stray references to Pipelines in the workspcae repo explorer
Jun 14 2021
This looks great! I think the iconography + colors are nice and I'm a fan of not having the icons click through to the sensor / schedule pages. Ideally we'll just lift the valuable things off those pages onto the Job overview.
Jun 10 2021
Jun 4 2021
Jun 3 2021
This looks great to me! Only other thing that occurs to me is that we also show streaming logs when you open a step's stderr | stdout panel from the bottom half of the run details view... I wonder if we need a similar fork over there?
- Test fixes
- Merge branch 'master' of github.com:dagster-io/dagster into abg/pipelineandmode
- Apply diff feedback
- Use a React hook to upgrade old urls to include mode, wrap no-snapshot-id behavior in the same way
May 26 2021
Hey @alangenfeld! Let me double check that the URLs without the mode work as expected - we can definitely maintain backwards compatibility. The only other change in this diff that isn't feature-flagged is that places where both pipeline mode and snapshot are listed (I believe just the run details header) have changed from pipeline@snapshot:mode to pipeline:mode@snapshot. I think the old style was chosen because the snapshot reflects the pipeline and then mode applies configuration, but I think the entire thing should be clickable and pipeline:mode go to the pipeline overview page and @snapshot goes to the snapshot-in-time view of the pipeline. Depending on how long we want to leave this feature flagged it could make sense to make that conditional as well but it seemed minor.
May 25 2021
Hey folks! I chatted with Sandy about this one and it sounds like we're all mostly in agreement that this doesn't reinforce a correct mental model of the system and we should probably omit this button and consider other ways to improve the linkage between assets and partition sets. Going to abandon this and follow up on the GH issue with the user!
May 24 2021
Patched this down and played with it a bit to make sure the useRefs are working properly, looks good to me!
Ahh good catch - this looks good to me
Looks good to me!
Wow this is more minimal than I would have expected actually!
May 19 2021
May 18 2021
Looks good to me! Definitely like wrapping the containers into a TestProvider
May 14 2021
Looks good to me and patches ok! Excited to give this a spin for a bit and see what else they've improved 🙌
May 10 2021
This looks great! Exciting to see a permissions layer coming together. I think it'd be nice to try making a container component that applies the disabled + tooltip but it's pretty clean already with hook approach!
May 7 2021
Hmm I think this works for now because we're planning on using the authToken primarily to attribute runs, etc. to different users, but I think if we really wanted security we'd need to add auth to the websockets interface as well (or stop using it), right? I wonder if we should add a small comment in here explaining that there are still un-authorized GraphQL requests being made from the app in the few places we need websockets?
Ahh yeah this will be much better! I'm not 100% sure when we started doing it the other way but I'm a fan of seeing them right next to the source in the file tree 👍
Add missing case triggered when run is half complete