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.

Sign up for GitHub

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

Jump to bottom

feat(cli): add more details to get cli #10815

Merged
merged 3 commits into from
Jul 3, 2024
Merged

feat(cli): add more details to get cli #10815

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ef2490
feat(cli): add more details to get cli
anshbansal Jul 1, 2024
9bd41d2
code review feedback
anshbansal Jul 2, 2024
5a69a8d
code review feedback
anshbansal Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/how/updating-datahub.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ This file documents any backwards-incompatible changes in DataHub and assists pe

- Protobuf CLI will no longer create binary encoded protoc custom properties. Flag added `-protocProp` in case this
anshbansal marked this conversation as resolved.
Show resolved Hide resolved
behavior is required.
- `datahub get ...` CLI output format changed a bit. To use the older output format pass `datahub get --no-details ...`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a downside to leaving the default to be the concise (--no-details) option? That way, we don't unintentionally break anyone who might be using this as an unofficial contract.

Copy link
Collaborator Author

@anshbansal anshbansal Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Most people won't find out the flag exists. We have a lot of CLI commands but because of not enough docs folks never discover they exist. I was hoping to make a more sane default.

But I can revert this breaking change. Not a hard requirmeent to do the breaking change.


### Potential Downtime

Expand Down
8 changes: 7 additions & 1 deletion metadata-ingestion/src/datahub/cli/cli_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,7 @@ def get_aspects_for_entity(
aspects: List[str],
typed: bool = False,
cached_session_host: Optional[Tuple[Session, str]] = None,
details: bool = False,
) -> Dict[str, Union[dict, _Aspect]]:
# Process non-timeseries aspects
non_timeseries_aspects = [a for a in aspects if a not in TIMESERIES_ASPECT_MAP]
Expand Down Expand Up @@ -553,7 +554,12 @@ def get_aspects_for_entity(
aspect_name
)

aspect_dict = a["value"]
if details:
aspect_dict = a
for k in ["name", "version", "type"]:
del aspect_dict[k]
else:
aspect_dict = a["value"]
if not typed:
aspect_map[aspect_name] = aspect_dict
elif aspect_py_class:
Expand Down
13 changes: 11 additions & 2 deletions metadata-ingestion/src/datahub/cli/get_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,17 @@ def get() -> None:
@get.command()
@click.option("--urn", required=False, type=str)
@click.option("-a", "--aspect", required=False, multiple=True, type=str)
@click.option(
"--details/--no-details",
required=False,
is_flag=True,
default=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

switch default to False

help="Whether to .",
Copy link
Contributor

Choose a reason for hiding this comment

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

Add doc string

)
@click.pass_context
@upgrade.check_upgrade
@telemetry.with_telemetry()
def urn(ctx: Any, urn: Optional[str], aspect: List[str]) -> None:
def urn(ctx: Any, urn: Optional[str], aspect: List[str], details: bool) -> None:
"""
Get metadata for an entity with an optional list of aspects to project.
This works for both versioned aspects and timeseries aspects. For timeseries aspects, it fetches the latest value.
Expand All @@ -39,7 +46,9 @@ def urn(ctx: Any, urn: Optional[str], aspect: List[str]) -> None:
logger.debug(f"Using urn from args {urn}")
click.echo(
json.dumps(
get_aspects_for_entity(entity_urn=urn, aspects=aspect, typed=False),
get_aspects_for_entity(
entity_urn=urn, aspects=aspect, typed=False, details=details
),
sort_keys=True,
indent=2,
)
Expand Down
Loading