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
feat(cli): support datahub ingest mcps
#7871
Conversation
@@ -233,6 +233,31 @@ def parse_restli_response(response): | |||
return rows | |||
|
|||
|
|||
@ingest.command() | |||
@click.argument("path", type=click.Path(exists=True)) | |||
def metadata_file(path: str) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works even if you point the source to a directory that contains one or more metadata event files. So the docs should reflect that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
""" | ||
Ingest from a metadata json file. | ||
|
||
This requires that you've run `datahub init` to set up your config. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't require it since sink is auto-inferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sink is inferred from your env variables or your ~/.datahubenv config - so they do need to run init first
@@ -233,6 +233,31 @@ def parse_restli_response(response): | |||
return rows | |||
|
|||
|
|||
@ingest.command() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit nervous about taking the top-level default option on this one even though I like the convenience of it.
Why should
datahub ingest <something>
default to ingesting an MCP file or directory with MCP files?
versus:
datahub ingest --source file <path>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my reading here is that we would have
datahub ingest --path <location>
Is that right? If yes, this feels fairly natural
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You use this as datahub ingest metadata-file my_file.json
$ datahub ingest metadata-file --help
Usage: datahub ingest metadata-file [OPTIONS] PATH
Ingest from a metadata json file or directory of files.
This requires that you've run `datahub init` to set up your config.
Options:
--help Show this message and exit.
datahub ingest metadata-file
datahub ingest mcps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Checklist