User Details
- User Since
- Mar 20 2019, 8:25 PM (219 w, 6 d)
- Roles
- Administrator
Aug 24 2021
Aug 19 2021
Aug 12 2021
what about all the other executors? Just fixing it for multiprocess seems wrong
I think the asymettery between solids and resources and their memoization behavior is unfortunate, but as long as its clearly communicated it should be fine
Aug 11 2021
Aug 9 2021
Aug 4 2021
I think Bens "Show Coverage" suggestion is better the "load" ones
we can build and show the link to the partitions page with [1] right?
I think its very valuable for this example to include input & output mapping
Aug 3 2021
Aug 2 2021
make sure we update the mypy type on StepFailureData which is also not set as Optional
what condition did we add where we have a step failure with no exception?
My initial reaction is that this is a pretty small improvement for a very specialized utility function. Its hard for me to make a strong case that this is any better than reconstructable in terms of having to discover some special tool when the normal testing flow fails on a dagstermill solid.
add tests for valid non-str tag and invalid non-str tag
Jul 29 2021
the inline comments for the tags bit is still an open area of concern
^ above comments worth considering
Jul 28 2021
agh - should have caught this in review
This strategy only really works for solids. A follow up diff will address the resources situation. Not sure if the best solution there is to keep the version argument on the resource decorator and make it not required to use memoization, or to add a resource_versioning_strategy argument (also not required) as well.
add env, test via 'DAGSTER_GRPC_MAX_RX_BYTES=5000000 dagit --grpc-port 5000'
Jul 27 2021
this looks good, but is back-compat a concern? if you upgrade dagit and don't upgrade the server will things break?
looking forward to a more thorough revamp of this stuff
Jul 26 2021
Additionally, having trouble getting the snapshot tests to cooperate. pytest --snapshot-update does not seem to be doing the trick.
the broad except + assumptions spooks me a bit, so at least make sure the assumptions are clearly commented in-case someone bumps in to this debugging
ah ok so it is the module load - the error condition you describe makes sense then but definitely worth commenting in [1] that we are assuming any exception that occurs is coming from the target code and due to python module loading behavior we capture, keep, and always re-throw that original error.
current recommendation is that, in their IOManager, users switch on the Python type attached to the InputDefinition
Jul 23 2021
rebase, review feedback
ya this is the right call
Well given that installing the pre-commit hooks is a choice, I think its safe to add the config file that meets our projects standards to ease that process for those that want it.
Jul 22 2021
round trip since there are failures any way - I think these last set of test adjustments should be it
welp - this wasnt quite right. Followed up with
https://github.com/dagster-io/dagster/commit/66a185240a7b7137d8fb97beaabd3b3d93ae9b4e
where i just disabled PRs as well for now
PRs might make that noisy-ish still but we may have to disable those as well
thanks! very clean change now
Everything thats here seems reasonable from my perspective
rebase