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

[dagster-hex] update to latest standards #28

Merged
merged 11 commits into from
Dec 16, 2024
Merged

Conversation

cmpadden
Copy link
Collaborator

@cmpadden cmpadden commented Dec 3, 2024

We are moving away from using resource decorated functions as resources, and instead favoring ConfigurableResource. This pull request "modernizes" the Hex integration to use a ConfigurableResource:

  • Use ConfigurableResource for HexResource
  • Remove legacy technique of defining resource hex_resource
  • Add unit test for using HexResource from an asset

Next steps will be updating the documentation, and flushing out more examples of using the resource.

Presently, I don't believe we need a hex_project_asset as we don't derive much for the user automatically, but can instead provide thorough examples of using the resource. This may change in the future, if we decide to generate assets for all projects in a workspace.


Unfinished tasks:

  • Update docstrings / README

@cmpadden cmpadden changed the title Colton/migrate dagster hex [dagster-hex] update to latest standards Dec 3, 2024
@cmpadden
Copy link
Collaborator Author

cmpadden commented Dec 3, 2024

@maximearmstrong what are your thoughts on removing hex_resource altogether?

@maximearmstrong
Copy link

maximearmstrong commented Dec 4, 2024

what are your thoughts on removing hex_resource altogether?

@cmpadden Very much in favor. I think deprecating the resource makes sense, then we can remove it when the breaking version is released.

cmpadden added a commit that referenced this pull request Dec 12, 2024
Copy link

@maximearmstrong maximearmstrong left a comment

Choose a reason for hiding this comment

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

Approved pending resolution of this thread

@cmpadden cmpadden merged commit 1623780 into main Dec 16, 2024
1 check passed
@cmpadden cmpadden deleted the colton/migrate-dagster-hex branch December 16, 2024 19:29
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.

2 participants