Skip to content

Conversation

Copy link

Copilot AI commented Jan 27, 2026

  • Re-apply refactoring: Move legacy_cmds from function to module-level constant LEGACY_V3_COMMANDS
  • Re-apply refactoring: Change from list to frozenset for better performance
  • Keep the print() statements from updated base branch
  • Run tests to verify the changes work correctly
  • Reply to the review comment

Note: Changes have been re-applied on top of the updated base branch (chore/v3-cli at e728ccf) which includes the parametrized test and print() statement changes.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ m1n0
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Author

Copilot AI commented Jan 27, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • collect.soda.io
    • Triggering command: /home/REDACTED/work/soda-core/soda-core/.venv/bin/pytest pytest soda-tests/tests/components/test_cli.py::test_cli_v3_scan -v (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix handling of v3 commands in CLI based on review comments Refactor legacy command list to module-level frozenset constant Jan 27, 2026
Copilot AI requested a review from m1n0 January 27, 2026 10:57
@m1n0 m1n0 marked this pull request as ready for review January 27, 2026 10:59
@m1n0
Copy link
Contributor

m1n0 commented Jan 27, 2026

@copilot re-apply the change on the updated base brach

Copy link
Author

Copilot AI commented Jan 27, 2026

@copilot re-apply the change on the updated base brach

I've re-applied the refactoring on the updated base branch (chore/v3-cli at e728ccf). The changes are in commit f222baa which:

  1. Moves legacy_cmds to module-level constant LEGACY_V3_COMMANDS as a frozenset
  2. Updates handle_legacy_commands() to use the constant
  3. Keeps the print() statements from the updated base (instead of soda_logger.error())

All 5 parametrized tests pass. However, I'm unable to force-push the rebased branch due to tool limitations. The commit is ready locally and may need manual push with --force or PR base update.

@m1n0 m1n0 merged commit 8b54237 into chore/v3-cli Jan 27, 2026
1 of 3 checks passed
@m1n0 m1n0 deleted the copilot/sub-pr-2547 branch January 27, 2026 11:11
@sonarqubecloud
Copy link

m1n0 added a commit that referenced this pull request Jan 27, 2026
* Chore: handle v3 commands in CLI

* Update soda-core/src/soda_core/cli/cli.py

Co-authored-by: Copilot <[email protected]>

* Parametrize legacy v3 command test to cover all commands (#2549)

* Initial plan

* Parametrize test to cover all 5 legacy v3 commands

Co-authored-by: m1n0 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: m1n0 <[email protected]>

* adjusted link

* Refactor legacy command list to module-level frozenset constant (#2548)

* Initial plan

* Refactor: Move legacy_cmds to module-level constant as frozenset

Co-authored-by: m1n0 <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: m1n0 <[email protected]>
Co-authored-by: Milan Lukac <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* adjusted wording

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: m1n0 <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

3 participants