flytekit icon indicating copy to clipboard operation
flytekit copied to clipboard
verified publisher icon flyteorg

[Feat] Enable registered entity summary output and quiet flag in pyflyte register

Open paullongtan opened this issue 11 months ago • 16 comments

Tracking issue

Closes flyteorg/flyte#3919

Why are the changes needed?

Currently, pyflyte register does not have an option to output registered entities to a file. This PR creates such functionality.

What changes were proposed in this pull request?

Pyflyte register

  • Add option --quiet to mute all outputs to CLI
  • Add option --summary-format to output registration summary to the CLI, uses json as default format and supports both yaml and json output format
  • Add new unit test

How was this patch tested?

  • [x] Unit testing
  • [x] Built and run some Flytekit test

Setup process

Screenshots

image image image image

Check all the applicable boxes

  • [x] I updated the documentation accordingly.
  • [x] All new and existing tests passed.
  • [x] All commits are signed-off.

Related PRs

Docs link

Summary by Bito

This pull request significantly enhances the `pyflyte register` command by adding options for outputting registered entities in JSON and YAML formats, along with a `--quiet` flag to suppress console output. It also introduces new agents for improved interactions and comprehensive unit tests to ensure the reliability of these features.

paullongtan avatar Jan 02 '25 00:01 paullongtan

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

welcome[bot] avatar Jan 02 '25 00:01 welcome[bot]

Code Review Agent Run Status

  • Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at [email protected].

flyte-bot avatar Jan 02 '25 00:01 flyte-bot

Code Review Agent Run Status

  • Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at [email protected].

flyte-bot avatar Jan 02 '25 01:01 flyte-bot

Code Review Agent Run #9a3edb

Actionable Suggestions - 3
  • flytekit/tools/repo.py - 2
    • Consider enhancing registration result information · Line 341-346
    • Consider using set for format checking · Line 402-404
  • flytekit/clis/sdk_in_container/register.py - 1
    • Consider validating summary format and directory · Line 146-159
Additional Suggestions - 1
  • tests/flytekit/unit/cli/pyflyte/test_register.py - 1
Review Details
  • Files reviewed - 3 · Commit Range: f51be71..9bba92f
    • flytekit/clis/sdk_in_container/register.py
    • flytekit/tools/repo.py
    • tests/flytekit/unit/cli/pyflyte/test_register.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

flyte-bot avatar Jan 02 '25 05:01 flyte-bot

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Feature Improvement - Enhanced Registration Output Format Support

- pyflyte.py - Added quiet flag support and improved output format handling

- register.py - Added summary-format and quiet options for registration output

- repo.py - Implemented JSON/YAML output formatting and quiet mode functionality
Testing - Registration Output Format Tests

- test_package.py - Added tests for package validation with verbose flag

- test_register.py - Added tests for JSON/YAML output formats and quiet mode

flyte-bot avatar Jan 02 '25 06:01 flyte-bot

Code Review Agent Run #a2cf6a

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 9bba92f..db9f744
    • tests/flytekit/unit/cli/pyflyte/test_register.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

flyte-bot avatar Jan 02 '25 07:01 flyte-bot

Nice first PR! I think you can add more unit tests.

  1. yaml
  2. testing registration without specifying --summary-dir

Thanks! I have added unit tests for both yaml and registration without specifying --summary-dir.

paullongtan avatar Jan 02 '25 17:01 paullongtan

Code Review Agent Run #5721dc

Actionable Suggestions - 3
  • flytekit/tools/repo.py - 1
    • Consider safer click.secho state management · Line 432-436
  • tests/flytekit/unit/cli/pyflyte/test_register.py - 2
Review Details
  • Files reviewed - 3 · Commit Range: db9f744..a3734e4
    • flytekit/clis/sdk_in_container/register.py
    • flytekit/tools/repo.py
    • tests/flytekit/unit/cli/pyflyte/test_register.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

flyte-bot avatar Jan 07 '25 07:01 flyte-bot

Code Review Agent Run #93cfad

Actionable Suggestions - 1
  • flytekit/tools/repo.py - 1
    • Consider consolidating duplicate error handling logic · Line 394-401
Review Details
  • Files reviewed - 3 · Commit Range: a3734e4..8a82a78
    • flytekit/clis/sdk_in_container/register.py
    • flytekit/tools/repo.py
    • tests/flytekit/unit/cli/pyflyte/test_register.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

flyte-bot avatar Jan 07 '25 09:01 flyte-bot

Code Review Agent Run #7297c4

Actionable Suggestions - 2
  • flytekit/clis/sdk_in_container/register.py - 1
    • Consider using context manager for logger · Line 37-37
  • tests/flytekit/unit/cli/pyflyte/test_register.py - 1
Review Details
  • Files reviewed - 3 · Commit Range: 8a82a78..05ab275
    • flytekit/clis/sdk_in_container/register.py
    • flytekit/tools/repo.py
    • tests/flytekit/unit/cli/pyflyte/test_register.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

flyte-bot avatar Jan 08 '25 07:01 flyte-bot

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.58%. Comparing base (4208a64) to head (4068ad0). Report is 13 commits behind head on master.

Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #3028       +/-   ##
===========================================
+ Coverage   51.81%   83.58%   +31.77%     
===========================================
  Files         202        3      -199     
  Lines       21469      195    -21274     
  Branches     2766        0     -2766     
===========================================
- Hits        11125      163    -10962     
+ Misses       9735       32     -9703     
+ Partials      609        0      -609

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 09 '25 05:01 codecov[bot]

New updates:

  1. Added option --quiet to pyflyte register to mute all output to CLI
  2. Updated registration summary to output directly to the CLI instead of saving to a file to ensure scalability
  • uses the --quiet flag to ensure proper output format
  • drops option --summary-dir for pyflyte register

Note:

  1. I have temporarily commented out the log for config file overwriting in pyflyte.py and log for packages used in validate_package function in utils.py. This is to prevent these logs from breaking the CLI output when outputting registration summary to the CLI and ensure no CLI outputs when using the --quiet flag for register.

paullongtan avatar Jan 09 '25 05:01 paullongtan

Code Review Agent Run #24c91a

Actionable Suggestions - 0
Review Details
  • Files reviewed - 4 · Commit Range: 05ab275..4b2b3b9
    • flytekit/clis/sdk_in_container/pyflyte.py
    • flytekit/clis/sdk_in_container/register.py
    • flytekit/clis/sdk_in_container/utils.py
    • tests/flytekit/unit/cli/pyflyte/test_register.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

flyte-bot avatar Jan 09 '25 05:01 flyte-bot

Code Review Agent Run #d1c6c9

Actionable Suggestions - 0
Additional Suggestions - 1
  • tests/flytekit/unit/cli/pyflyte/test_package.py - 1
    • Consider improving click context initialization · Line 181-183
Review Details
  • Files reviewed - 3 · Commit Range: 4b2b3b9..43704b8
    • flytekit/clis/sdk_in_container/pyflyte.py
    • flytekit/clis/sdk_in_container/utils.py
    • tests/flytekit/unit/cli/pyflyte/test_package.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by Bito Logo

flyte-bot avatar Jan 28 '25 00:01 flyte-bot

Code Review Agent Run Status

  • Limitations and other issues: ❌ Failure - Bito Code Review Agent didn't review this pull request automatically because it exceeded the size limit. No action is needed if you didn't intend for the agent to review it. Otherwise, you can initiate the review by typing /review in a comment below.

flyte-bot avatar Jan 28 '25 04:01 flyte-bot

Code Review Agent Run Status

  • Limitations and other issues: ❌ Failure - Bito Code Review Agent didn't review this pull request automatically because it exceeded the size limit. No action is needed if you didn't intend for the agent to review it. Otherwise, you can initiate the review by typing /review in a comment below.

flyte-bot avatar Mar 27 '25 10:03 flyte-bot