[DCP - CLI] Update the CLI to specify the --import flag - #99
Conversation
* Added datacommons-cli and datacommons-admin readmes * updated installation section
…ref / tag (#94) * Updated admin init command to fetch modules with the current version ref / tag * reverted readme changes * feedback * fmt * bug fix
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several updates to the Data Commons Platform administrative and CLI tools, including the addition of an --import option to the ingestion start command, corresponding Cloud Run container overrides, updated Terraform IAM configurations, and expanded documentation. Review feedback recommends adding type annotations (str | None) to the new import_name parameters in ingest_cli.py and ingestion_job_client.py, as well as implementing unit tests to verify the parsing and payload behavior of the new --import flag.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces CLI support for specifying specific imports to run during data ingestion, updates the Terraform templates to use version-prefixed Git refs by default, and adds comprehensive documentation for the admin and CLI packages. The reviewer feedback highlights a few minor issues: a typo in the README CLI command option (--import instead of --imports), a missing type hint for the new imports parameter in ingest_cli.py, and an outdated API key comment in a newly added test.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to trigger data ingestion jobs with specific imports via the datacommons CLI. It updates the CLI and the underlying ingestion_job_client to pass --mode=dcpbridge and --imports as container overrides to the Cloud Run job, adds corresponding unit tests, and grants the workflow service account the roles/run.developer role. Feedback on these changes suggests retaining the default DATA_RUN_MODE environment variable in the Terraform configuration to maintain backward compatibility for other triggers, and correcting the type hint for the imports parameter in start_job to str | None.
…ent.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the legacy data job to support the --imports flag. It has backwards compatibility, if --imports is not provided, we will continue looking for the config.json in the input/dir. Otherwise, we will be looking in the subfolders provided. CLI is updated in datacommonsorg/datacommons#99 Note that this supports following thepath in GCS to make the provenance name, but it does not yet support per-import feature yet. We currently hack te provenance name of multiple different imports. The work needed to do this blew up the PR, so I am now splitting it up. This is JUST the import job changes for support a --imports flag and fetching the configs. The follow up PR will add full support for multiple individual imports.
2 participants
No description provided.