Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle partitions natively in W&B IO Manager #15170

Merged
merged 11 commits into from
Aug 9, 2023

Conversation

chrishiste
Copy link
Contributor

Summary & Motivation

This PR aims to enrich the W&B integration by enhancing the functionality and user experience of the IO manager. We've focused on native handling of (multi-)partitioned assets, which streamlines the process and ensures our implementation is more robust. We also addressed and resolved several bugs, to provide a more seamless experience to users.

In our commitment to clarity and ease of use, we've added comprehensive examples showcasing the use of partitions and multi-partitions, as well as how to specify a version or alias in the configuration metadata.

We’ve also given much-needed attention to user-facing errors, rewording them for better understanding and visibility. Moreover, we introduced additional errors to alert users when there might be issues with their provided configuration, improving their ability to self-diagnose and rectify issues.

How I Tested These Changes

The examples added were run in multiple scenarios to ensure that they cover common use cases and successfully demonstrate the intended functionality.

We also replicated common bugs that were reported, and confirmed that the fixes eliminated these issues.

Lastly, we ensured backward compatibility and non-disruptive deployment with integration tests.

We're excited to share these updates and we believe these improvements will significantly enhance the user's experience. Looking forward to your feedback, Dagster team!

@jamiedemaria jamiedemaria requested a review from yuhan July 7, 2023 19:31
@jamiedemaria
Copy link
Contributor

I think @yuhan will be best situated to review this PR, but she is out on vacation until mid next week!

},
output_required=False,
)
def read_objects_directly(context, partitions):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trying to wrap my head around here. where does the partitions come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the AssetIn

@yuhan
Copy link
Contributor

yuhan commented Jul 19, 2023

the test is mostly green except for the formatting/linting. could you run make ruff to lint the code to be consistent with the rest of codebase?

@chrishiste
Copy link
Contributor Author

@yuhan Code is linted, I had to pull the latest master to get the new config :)

Update this repo with the latest master of dagster repo
@yuhan
Copy link
Contributor

yuhan commented Aug 9, 2023

The build failure should have been resolved on master. I tested this PR + latest master here: #15758 the build is green.

@yuhan yuhan merged commit d76c2e5 into dagster-io:master Aug 9, 2023
@chrishiste chrishiste deleted the wandb-integration-bugfix branch August 9, 2023 12:56
PedramNavid pushed a commit that referenced this pull request Sep 12, 2023
## Summary & Motivation

This PR aims to enrich the W&B integration by enhancing the
functionality and user experience of the IO manager. We've focused on
native handling of (multi-)partitioned assets, which streamlines the
process and ensures our implementation is more robust. We also addressed
and resolved several bugs, to provide a more seamless experience to
users.

In our commitment to clarity and ease of use, we've added comprehensive
examples showcasing the use of partitions and multi-partitions, as well
as how to specify a version or alias in the configuration metadata.

We’ve also given much-needed attention to user-facing errors, rewording
them for better understanding and visibility. Moreover, we introduced
additional errors to alert users when there might be issues with their
provided configuration, improving their ability to self-diagnose and
rectify issues.

## How I Tested These Changes

The examples added were run in multiple scenarios to ensure that they
cover common use cases and successfully demonstrate the intended
functionality.

We also replicated common bugs that were reported, and confirmed that
the fixes eliminated these issues.

Lastly, we ensured backward compatibility and non-disruptive deployment
with integration tests.

We're excited to share these updates and we believe these improvements
will significantly enhance the user's experience. Looking forward to
your feedback, Dagster team!

---------

Co-authored-by: yuhan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants