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] dg info -> dg inspect #27955

Merged
merged 1 commit into from
Feb 21, 2025
Merged

Conversation

smackesey
Copy link
Collaborator

@smackesey smackesey commented Feb 20, 2025

Summary & Motivation

Rename dg info -> dg inspect (verb, more general).

How I Tested These Changes

Existing test suite.

Copy link
Collaborator Author

smackesey commented Feb 20, 2025

@smackesey smackesey force-pushed the sean/components/info-inspect branch from 6d3e165 to 93422de Compare February 20, 2025 19:41
@smackesey smackesey force-pushed the sean/components/move-cli-to-verb-first-tests branch from d67b573 to 42ab9f5 Compare February 20, 2025 19:41
Base automatically changed from sean/components/move-cli-to-verb-first-tests to master February 20, 2025 19:41
@smackesey smackesey marked this pull request as ready for review February 20, 2025 19:42
@smackesey smackesey force-pushed the sean/components/info-inspect branch from 93422de to 81f7e2f Compare February 20, 2025 19:43
@smackesey smackesey force-pushed the sean/components/info-inspect branch from 81f7e2f to 9c255b3 Compare February 20, 2025 19:49
@smackesey smackesey requested a review from neverett as a code owner February 20, 2025 19:49
Copy link
Member

@schrockn schrockn left a comment

Choose a reason for hiding this comment

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

We should indicate in the help string that is design for consumption by tools rather than by humans.

@smackesey
Copy link
Collaborator Author

We should indicate in the help string that is design for consumption by tools rather than by humans.

At the moment it is because we dump raw JSON schema but I think it should be changed to be human-friendly by default and have a --json option.

Copy link
Member

My thought is that we should focus all our effort on human friendly docs rather than maintain another surface area.

Copy link
Member

But I'm open to persuasion here.

Copy link
Collaborator Author

I imagine inspect being used to target not just component-types, but projects, deployments, whatever. Some of these items might not be part of generated docs. I also the ability to get human-friendly CLI inspection of domain objects is just a basic part of a first class CLI (see e.g. docker inspect). It's annoying to always need to context-switch to the browser and possibly have to navigate in it.

@schrockn
Copy link
Member

Ok got. Can you give me an example of what you think a human-readable version of inspect would look like for, say, a component type?

@smackesey
Copy link
Collaborator Author

smackesey commented Feb 21, 2025

Can you give me an example of what you think a human-readable version of inspect would look like for, say, a component type?

Something like this. We can produce a greatly simplified/compact version of the JSON schema that maybe just doesn't show nested objects at all, or maybe some compromise simple representation

simple_pipes_script_asset@dagster_components.test

A simple asset that runs a Python script with the Pipes subprocess client.

Because it is a pipes asset, no value is returned.

Scaffold attributes:
  asset_key*: string
  filename*: string
  some_other_field: <object>

Component attributes:
  asset_key*: string
  filename*: string

(run with `--json` for full represenation of schema

Can throw in some coloring as well.

@schrockn
Copy link
Member

schrockn commented Feb 21, 2025 via email

@smackesey smackesey merged commit d0e767d into master Feb 21, 2025
7 checks passed
@smackesey smackesey deleted the sean/components/info-inspect branch February 21, 2025 17:54
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