Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: remove labels from an execution #7256

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

fix: remove labels from an execution #7256

wants to merge 7 commits into from
+83 −4

Conversation

Malaydewangan09
Copy link
Contributor

What changes are being made and why?

Addressed an issue where labels could not be removed from an execution. Updated the backend logic to handle label deletions correctly.

Fixes #6205.

@Malaydewangan09 Malaydewangan09 marked this pull request as ready for review February 11, 2025 16:26
@Ben8t Ben8t requested a review from loicmathieu March 6, 2025 14:23
loicmathieu
loicmathieu requested changes Mar 6, 2025
View reviewed changes
Copy link
Member

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

@Malaydewangan09 can you add a test to your PR?

@Malaydewangan09 Malaydewangan09 force-pushed the fix/remove-labels-from-execution branch from fbbbe6f to 9b1a9d6 Compare March 7, 2025 06:42
@loicmathieu
Copy link
Member

I think this is more complex than that as:

  • We need to be able to remove/update/add labels
  • We need to not be able to remove/update/add system labels
  • System labels should always be the same as only the system (Kestra) can add them

@Malaydewangan09
Copy link
Contributor Author

Malaydewangan09 commented Mar 11, 2025 •
edited
Loading

@loicmathieu , Agree!

I've also added a test for system labels along with the existing test, which was removing system labels.
Now:

  • We can add/update/remove labels.
  • System labels cannot be added/updated/removed.
  • System labels remain unchanged, as only Kestra can manage them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loicmathieu loicmathieu Awaiting requested review from loicmathieu

Assignees
No one assigned
Labels
None yet
Projects
Pull Requests
Status: To review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Labels cannot be removed from an execution
3 participants
@Malaydewangan09 @loicmathieu @MilosPaunovic