Skip to content

Commit

Permalink
respond to review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
benpankow committed May 9, 2024
1 parent d74bc8b commit f82332b
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 20 deletions.
4 changes: 4 additions & 0 deletions python_modules/dagster/dagster/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,14 @@
)
from dagster._core.definitions.metadata import (
BoolMetadataValue as BoolMetadataValue,
CodeReferencesMetadataValue as CodeReferencesMetadataValue,
DagsterAssetMetadataValue as DagsterAssetMetadataValue,
DagsterJobMetadataValue as DagsterJobMetadataValue,
DagsterRunMetadataValue as DagsterRunMetadataValue,
FloatMetadataValue as FloatMetadataValue,
IntMetadataValue as IntMetadataValue,
JsonMetadataValue as JsonMetadataValue,
LocalFileCodeReference as LocalFileCodeReference,
MarkdownMetadataValue as MarkdownMetadataValue,
MetadataEntry as MetadataEntry,
MetadataValue as MetadataValue,
Expand All @@ -284,7 +286,9 @@
TableSchemaMetadataValue as TableSchemaMetadataValue,
TextMetadataValue as TextMetadataValue,
TimestampMetadataValue as TimestampMetadataValue,
UrlCodeReference as UrlCodeReference,
UrlMetadataValue as UrlMetadataValue,
link_to_source_control as link_to_source_control,
with_source_code_references as with_source_code_references,
)
from dagster._core.definitions.metadata.table import (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
CodeReferencesMetadataSet as CodeReferencesMetadataSet,
CodeReferencesMetadataValue as CodeReferencesMetadataValue,
LocalFileCodeReference as LocalFileCodeReference,
SourceControlCodeReference as SourceControlCodeReference,
UrlCodeReference as UrlCodeReference,
link_to_source_control as link_to_source_control,
with_source_code_references as with_source_code_references,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ class LocalFileCodeReference(DagsterModel):

@experimental
@whitelist_for_serdes
class SourceControlCodeReference(DagsterModel):
"""Represents a source code location ."""
class UrlCodeReference(DagsterModel):
"""Represents a source location which points at a URL, for example
in source control.
"""

source_control_url: str
line_number: int
label: Optional[str] = None


Expand All @@ -64,7 +65,7 @@ class CodeReferencesMetadataValue(DagsterModel, MetadataValue["CodeReferencesMet
references to source control.
"""

code_references: List[Union[LocalFileCodeReference, SourceControlCodeReference]]
code_references: List[Union[LocalFileCodeReference, UrlCodeReference]]

@property
def value(self) -> "CodeReferencesMetadataValue":
Expand Down Expand Up @@ -124,9 +125,7 @@ def _with_code_source_single_definition(
existing_source_code_metadata = CodeReferencesMetadataSet.extract(
metadata_by_key.get(key, {})
)
sources_for_asset: List[
Union[LocalFileCodeReference, SourceControlCodeReference]
] = [
sources_for_asset: List[Union[LocalFileCodeReference, UrlCodeReference]] = [
*existing_source_code_metadata.code_references.code_references,
*sources,
]
Expand All @@ -147,14 +146,13 @@ def convert_local_path_to_source_control_path(
base_source_control_url: str,
repository_root_absolute_path: str,
local_path: LocalFileCodeReference,
) -> SourceControlCodeReference:
) -> UrlCodeReference:
source_file_from_repo_root = os.path.relpath(
local_path.file_path, repository_root_absolute_path
)

return SourceControlCodeReference(
source_control_url=f"{base_source_control_url}/{source_file_from_repo_root}",
line_number=local_path.line_number,
return UrlCodeReference(
source_control_url=f"{base_source_control_url}/{source_file_from_repo_root}#L{local_path.line_number}",
label=local_path.label,
)

Expand All @@ -178,7 +176,7 @@ def _convert_local_path_to_source_control_path_single_definition(
existing_source_code_metadata = CodeReferencesMetadataSet.extract(
metadata_by_key.get(key, {})
)
sources_for_asset: List[Union[LocalFileCodeReference, SourceControlCodeReference]] = [
sources_for_asset: List[Union[LocalFileCodeReference, UrlCodeReference]] = [
convert_local_path_to_source_control_path(
base_source_control_url,
repository_root_absolute_path,
Expand Down Expand Up @@ -215,11 +213,30 @@ def link_to_source_control(
source_control_branch: str,
repository_root_absolute_path: Union[Path, str],
) -> Sequence[Union["AssetsDefinition", "SourceAsset", "CacheableAssetsDefinition"]]:
"""Wrapper function which converts local file path code references to source control URLs
based on the provided source control URL and branch.
Args:
assets_defs (Sequence[Union[AssetsDefinition, SourceAsset, CacheableAssetsDefinition]]):
The asset definitions to which source control metadata should be attached.
Only assets with local file code references (such as those created by
`with_source_code_references`) will be converted.
source_control_url (str): The base URL for the source control system. For example,
"https://github.com/dagster-io/dagster".
source_control_branch (str): The branch in the source control system, such as "master".
repository_root_absolute_path (Union[Path, str]): The absolute path to the root of the
repository on disk. This is used to calculate the relative path to the source file
from the repository root and append it to the source control URL.
"""
if "gitlab.com" in source_control_url:
source_control_url = _build_gitlab_url(source_control_url, source_control_branch)
else:
# assume GitHub URL scheme
elif "github.com" in source_control_url:
source_control_url = _build_github_url(source_control_url, source_control_branch)
else:
raise ValueError(
"Invalid `source_control_url`."
" Only GitHub and GitLab are supported for linking to source control at this time."
)

return [
_convert_local_path_to_source_control_path_single_definition(
Expand All @@ -235,7 +252,8 @@ def link_to_source_control(
def with_source_code_references(
assets_defs: Sequence[Union["AssetsDefinition", "SourceAsset", "CacheableAssetsDefinition"]],
) -> Sequence[Union["AssetsDefinition", "SourceAsset", "CacheableAssetsDefinition"]]:
"""Wrapper function which attaches source code metadata to the provided asset definitions.
"""Wrapper function which attaches local code reference metadata to the provided asset definitions.
This points to the filepath and line number where the asset body is defined.
Args:
assets_defs (Sequence[Union[AssetsDefinition, SourceAsset, CacheableAssetsDefinition]]):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from dagster import AssetsDefinition, load_assets_from_modules
from dagster._core.definitions.metadata import (
LocalFileCodeReference,
SourceControlCodeReference,
UrlCodeReference,
link_to_source_control,
with_source_code_references,
)
Expand Down Expand Up @@ -125,15 +125,15 @@ def test_asset_code_origins_source_control() -> None:

assert isinstance(
asset.metadata_by_key[key]["dagster/code_references"].code_references[-1],
SourceControlCodeReference,
UrlCodeReference,
)
meta = cast(
SourceControlCodeReference,
UrlCodeReference,
asset.metadata_by_key[key]["dagster/code_references"].code_references[-1],
)

assert meta.source_control_url == (
"https://github.com/dagster-io/dagster/tree/master/python_modules/dagster"
+ (expected_file_path[len(DAGSTER_PACKAGE_PATH) :])
+ f"#L{expected_line_number}"
)
assert meta.line_number == int(expected_line_number)

0 comments on commit f82332b

Please sign in to comment.