Allow io manager keys to be str enums not just strs #18778
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
More details in this issue : #18623
Previously, Dagster didn't accept to serialize enums from the StrEnum library.
I modified the _pack_value() method so that if it receives an Enum that is also of type str, we return the value of this enum.
With this change, users will be able to define keys for IOManager, Jobs, Assets etc into a single file separate in scope through the name of the enum, just like this example :
How I Tested These Changes
I tested that it was possible to serialize and de-serialize an StrEnum and that the resulting de-serialized object was the str value of the enum (which is an ok behaviour for me because I will never read back the StrEnum in my use cases).
I also rerun all test from the
test_serdes.py
to check if it was okayI also tested the test provided in the issue but I didn't added to the code because it was maybe a too big test
To make the test I had to add strenum as extra-dependency for the tests, is it ok?