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.

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

Labels cannot be removed from an execution #6205

Open
fhussonnois opened this issue Nov 29, 2024 · 14 comments · May be fixed by #7256
Open

Labels cannot be removed from an execution #6205

fhussonnois opened this issue Nov 29, 2024 · 14 comments · May be fixed by #7256
Assignees
Labels
area/backend Needs backend code changes bug Something isn't working

Comments

@fhussonnois
Copy link
Member

Describe the issue

In the execution view, Set labels, removing a label doesn't work

Environment

  • Kestra Version: develop
@fhussonnois fhussonnois added bug Something isn't working area/backend Needs backend code changes labels Nov 29, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Issues Nov 29, 2024
@MilosPaunovic
Copy link
Member

Also, as @loicmathieu worked on labels few days ago, maybe he can give us some hints here.

@loicmathieu loicmathieu removed their assignment Dec 3, 2024
@SayedQassim
Copy link
Contributor

Hi @loicmathieu, Can you assign this to me?

@MilosPaunovic
Copy link
Member

I can assign you, but this might be a bit tricky to do, as we need to change a behavior a bit more than described here.

May I suggest that you check our curated list of good first issues to see if there is something you find interesting enough and think you can tackle independently. It can be further filtered by desired labels to narrow down the results (suggesting to filter by either area/backend or area/frontend ones, based on your preferences).

@SayedQassim
Copy link
Contributor

Hi @MilosPaunovic , I see, actually I did go through it yesterday and I kind of found the issue, as if we remove it from the frontend it get removed successfully and sent to the backend, the issue is that all labels is saved in one variable in the backend and the function that update the labels always go through the saved labels in the backend and add them again. I do not get it why it is built like that.

@SayedQassim
Copy link
Contributor

Anyway if you believe it is a bit complex I will look for a different issue.

@Ben8t
Copy link
Member

Ben8t commented Jan 21, 2025

Still valide on preview env as 21/01/2025

@Malaydewangan09
Copy link
Contributor

Hey, @MilosPaunovic
Can I pick this up?

@MilosPaunovic
Copy link
Member

Absolutely, go for it @Malaydewangan09! 🚀

@Malaydewangan09
Copy link
Contributor

Hey, @MilosPaunovic
This is the current implementation

Map<String, String> newLabels = labels.stream().collect(Collectors.toMap(Label::key, Label::value));
        if (execution.getLabels() != null) {
            execution.getLabels().forEach(
                label -> {
                    // only add execution label if not updated
                    if (!newLabels.containsKey(label.key())) {
                        newLabels.put(label.key(), label.value());
                    }
                }
            );
        }

Removing the condition resolves the issue.
Am I overlooking something? As you mentioned, this might be more tricky than it seems.
Let me know your thoughts. Thanks!

@MilosPaunovic
Copy link
Member

@loicmathieu We'd need your input here.

@MilosPaunovic MilosPaunovic moved this from Backlog to In progress in Issues Feb 4, 2025
@Malaydewangan09
Copy link
Contributor

Hey @loicmathieu , just following up. Let me know when you get a chance.
Thanks!

@loicmathieu
Copy link
Member

@Malaydewangan09 I'm not sure which condition you are talking about so the best would be to craft a reproducer as a unit test and to open a PR that fixes it.

@Malaydewangan09
Copy link
Contributor

Sure @loicmathieu

Malaydewangan09 added a commit to Malaydewangan09/kestra that referenced this issue Feb 8, 2025
Malaydewangan09 pushed a commit to Malaydewangan09/kestra that referenced this issue Feb 8, 2025
Malaydewangan09 added a commit to Malaydewangan09/kestra that referenced this issue Feb 8, 2025
@Malaydewangan09 Malaydewangan09 linked a pull request Feb 8, 2025 that will close this issue
@MilosPaunovic MilosPaunovic moved this from In progress to In review in Issues Feb 20, 2025
@MilosPaunovic
Copy link
Member

MilosPaunovic commented Feb 25, 2025

I've removed the necessary restriction for label removal on the UI side, but them are not being updated on the server as expected. @loicmathieu when you get the chance, could you review the PR from Malay and see if it does the trick?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backend Needs backend code changes bug Something isn't working
Projects
Status: In review
Development

Successfully merging a pull request may close this issue.

6 participants