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

[components] Rename build_defs_from_toplevel_components_folder to build_component_defs and make it less magical #26618

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

schrockn
Copy link
Member

@schrockn schrockn commented Dec 20, 2024

Summary & Motivation

Does a couple things

  • Renames the top-level entry point from build_defs_from_toplevel_components_folder to build_component_defs
  • Changes the scaffolding to make it a top-level export of dagster_components
  • Removes upwards traversal in the folder hiearchy. Instead requires targeting of the exact directory. This avoids magic and makes errors more explicit

How I Tested These Changes

Generated empty code location from scaffolding and loaded it.

Changelog

BK

Copy link
Member Author

schrockn commented Dec 20, 2024

@schrockn schrockn marked this pull request as ready for review December 20, 2024 02:45
@schrockn schrockn changed the title Rename build_defs_from_toplevel_components_folder to build_component_defs and make it less magical [components] Rename build_defs_from_toplevel_components_folder to build_component_defs and make it less magical Dec 20, 2024
Copy link
Contributor

@OwenKephart OwenKephart left a comment

Choose a reason for hiding this comment

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

note comment

def build_defs_from_toplevel_components_folder(
path: Path,
def build_component_defs(
code_location_path: Path,
Copy link
Contributor

Choose a reason for hiding this comment

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

code_location_path -> code_location_root to match docstring (could be swayed either direction but somewhat prefer code_location_root)

@schrockn schrockn force-pushed the schrockn/make-components-loading-less-magical branch from cdc62de to ef4ddd1 Compare December 20, 2024 17:51
Copy link
Member Author

schrockn commented Dec 20, 2024

Merge activity

  • Dec 20, 1:21 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Dec 20, 1:22 PM EST: Graphite rebased this pull request as part of a merge.
  • Dec 20, 1:23 PM EST: Graphite rebased this pull request as part of a merge.
  • Dec 20, 1:24 PM EST: A user merged this pull request with Graphite.

@schrockn schrockn force-pushed the schrockn/make-components-loading-less-magical branch from ef4ddd1 to 7c83f99 Compare December 20, 2024 18:21
@schrockn schrockn force-pushed the schrockn/make-components-loading-less-magical branch from 7c83f99 to cecbfdb Compare December 20, 2024 18:22
@schrockn schrockn merged commit 4324c56 into master Dec 20, 2024
1 check was pending
@schrockn schrockn deleted the schrockn/make-components-loading-less-magical branch December 20, 2024 18:24
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