Skip to content

fix(cssc): 31694717 'delete' command should check and prompt to cancel any running scan or patch task#34

Merged
cegraybl merged 11 commits into
feature/cssc_extfrom
cegraybl/cssc_31694717_delete_cancelruns
Mar 18, 2025
Merged

fix(cssc): 31694717 'delete' command should check and prompt to cancel any running scan or patch task#34
cegraybl merged 11 commits into
feature/cssc_extfrom
cegraybl/cssc_31694717_delete_cancelruns

Conversation

@cegraybl
Copy link
Copy Markdown

This change allows the 'delete' command to check and prompt to cancel any running scan or patch task before deleting the workflow. the operation will not prompt id used with the parameter --yes|-y and accepts --dryrun which will run the command without executing the cancellation of the tasks.
Includes small fixes for style and lint issues.

image

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that the 'delete' command checks for any currently running workflow tasks and, if found, prompts the user to cancel them before deletion proceeds. Key changes include:

  • Adding a new "yes" parameter to the delete_continuous_patch_v1 function so that the prompt can be bypassed.
  • Introducing logic to obtain running task runs and conditionally cancel them.
  • Updating relevant tests and associated function calls to support these changes.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
src/acrcssc/azext_acrcssc/helper/_taskoperations.py Modified delete_continuous_patch_v1 to accept a "yes" parameter and added logic to check and cancel running tasks; introduced _cancel_task_runs with optional dryrun behavior.
src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py Updated tests to check the new parameter usage and ensure cancellation logic is invoked.
src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py Updated deletion function calls to correctly pass the "yes" parameter.
Comments suppressed due to low confidence (1)

src/acrcssc/azext_acrcssc/helper/_taskoperations.py:300

  • Consider wrapping the cancellation request within a try-except block in the _cancel_task_runs function to ensure that a failure to cancel one task does not prevent subsequent cancellations.
for task in running_tasks:

Comment thread src/acrcssc/azext_acrcssc/helper/_taskoperations.py Outdated
Comment thread src/acrcssc/azext_acrcssc/helper/_taskoperations.py
@cegraybl cegraybl force-pushed the cegraybl/cssc_31694717_delete_cancelruns branch from cccf2ee to 8337639 Compare March 14, 2025 18:17
@cegraybl cegraybl requested a review from Copilot March 14, 2025 20:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the deletion operations for continuous patch tasks by removing the dry-run flag and replacing it with a confirmation prompt via the yes parameter. The changes include:

  • Removing dryrun parameters and associated handling in delete_continuous_patch_v1, _delete_task, and _delete_task_role_assignment.
  • Updating the oci artifact deletion logic and its tests to always perform deletion.
  • Adjusting test cases and function calls to use the new yes parameter.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

File Description
src/acrcssc/azext_acrcssc/helper/_taskoperations.py Updated deletion functions to remove dryrun logic and added explicit cancellation prompt handling
src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py Removed dryrun parameter from artifact deletion and enforced yes=True in the repository delete call
src/acrcssc/azext_acrcssc/tests/latest/* Updated tests for delete operations to match new function signatures and behavior
src/acrcssc/azext_acrcssc/cssc.py Adjusted the delete_acrcssc call to pass the yes parameter

Comment thread src/acrcssc/azext_acrcssc/helper/_taskoperations.py
@cegraybl cegraybl requested a review from Copilot March 14, 2025 22:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the deletion command so that it now checks for and prompts to cancel any running scan or patch tasks before deleting workflows, while also addressing minor style and lint issues. Key changes include:

  • Renaming the deletion function parameter from "dryrun" to "yes" for clarity.
  • Removing dry-run logic from task deletions and role assignment deletion, ensuring that cancellation prompts are used instead.
  • Adjusting tests accordingly to reflect these changes.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

File Description
src/acrcssc/azext_acrcssc/helper/_taskoperations.py Updated deletion flow for continuous patch tasks, including parameter renaming and improved error handling for individual task deletions.
src/acrcssc/azext_acrcssc/cssc.py Updated deletion command call to pass the correct parameter and removed dryrun from the API call.
src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py Removed the dryrun parameter from the deletion function to always execute deletion.
src/acrcssc/azext_acrcssc/tests/latest/* Updated tests to use the new parameter and removal of dryrun-based logic.
Comments suppressed due to low confidence (2)

src/acrcssc/azext_acrcssc/helper/_taskoperations.py:148

  • The function parameter has been renamed from 'dryrun' to 'yes'. Ensure that all related documentation, comments, and test cases are updated to reflect this change.
def delete_continuous_patch_v1(cmd, registry, yes):

src/acrcssc/azext_acrcssc/helper/_taskoperations.py:434

  • The removal of the dryrun check in _delete_task means deletions will be executed unconditionally. Confirm that this behavior is intentional and that any dry-run functionality is appropriately managed elsewhere.
def _delete_task(cmd, registry, task_name):

Comment thread src/acrcssc/azext_acrcssc/helper/_taskoperations.py
Copy link
Copy Markdown

@Ruchii-27 Ruchii-27 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread src/acrcssc/azext_acrcssc/helper/_taskoperations.py
Comment thread src/acrcssc/azext_acrcssc/helper/_taskoperations.py
@cegraybl cegraybl merged commit 856a6b0 into feature/cssc_ext Mar 18, 2025
@cegraybl cegraybl deleted the cegraybl/cssc_31694717_delete_cancelruns branch April 23, 2025 17:10
cegraybl added a commit that referenced this pull request Apr 23, 2025
…l any running scan or patch task (#34)

This change allows the 'delete' command to check and prompt to cancel
any running scan or patch task before deleting the workflow. the
operation will not prompt id used with the parameter `--yes|-y` and
accepts `--dryrun` which will run the command without executing the
cancellation of the tasks.
Includes small fixes for style and lint issues.

![image](https://github.com/user-attachments/assets/2d14adf6-617c-4aae-b177-45db995fbd93)
mabelegba pushed a commit that referenced this pull request Nov 25, 2025
* Upstream Merge

* Merge from workload-orchestration : Added Example in Description (#34)

* Merge from workload-orchestration : Fixed Example

* Fixed Example for SolutionTemplateVersion

* Made Changes for command-change CI fix'

* Reset Version 1.0.0b1

* Added Bulk and Diagnostics Back (#35)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Bulk Solution Example (#36)

* Added Bulk and Diagnostics Back

* Changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Fixed Solution Template Linter Issue

* Added Integration Tests Framework for WorkloadOrchestration #37

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added ServiceName  (#38)

* Added Tests

* Made Changes on ServiceName

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Full Target Solution Tests (#39)

* Added Tests

* Made Changes on ServiceName

* Nit Changes

* Made Changes for ContextLookup

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added E2E workflow Tests (#40)

* Added Tests

* Made Changes on ServiceName

* Nit Changes

* Made Changes for ContextLookup

* Added complete workflow

* Add Licence

* Added License

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Readme (#41)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Bulk Deployemnent LRO changes (#42)

* made changes

* Added change

* Added Changes in Test

* Added CLi Changes

* Added Setup

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Added Changes For Backward Compitablity (#43)

* Added Changes For Backward Compitablity

* Made changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Made changes (#44)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Target Operations API refractoring  (#47)

* Changes I made

* Make changes

* Made chnages

* Make changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Stable API 2025-06-01 (#48)

* Made changes

* Made Changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Change in ID in Targets (#50)

* Made changes

* changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Remove Id and Name from Target Review  (#51)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* final bulk (#52)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Changes in ARG

* Added Linter Exception

* Added ITTests (#53)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Standardize CLI (#54)

* Made Changes

* Made Changes

* Made changes

* Added Change

* Made changes

* Made changes

* Made changes

* Made Changes

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Linter FIxes and Remove Preview

* Removed Resolved from CLI

* change in version

* made changes (#55)

Co-authored-by: Atharva Udapure <audapure@microsoft.com>

* Fixed history

---------

Co-authored-by: Atharva Udapure <audapure@microsoft.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.

4 participants