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

Handle multiple jobs with the same name #4825

Merged
merged 2 commits into from
Feb 9, 2025
Merged

Handle multiple jobs with the same name #4825

merged 2 commits into from
Feb 9, 2025

Conversation

garrettjstevens
Copy link
Collaborator

The JobsListWidget treats the name of a job as an identifier, but also allowed multiple jobs with the same name. This changes the behavior so that if a job is added with the same name as an existing job, it skips adding the job and returns the existing one. There are other ways this could be addressed (e.g. assigning generated ids to jobs), but this seemed the least disruptive.

It also fixes a bug where if a job name that didn't exist was provided to removeJob or removedQueuedJob, it would remove the last job in the list. Now it doesn't remove any jobs and returns undefined.

@garrettjstevens garrettjstevens self-assigned this Feb 7, 2025
@cmdcolin cmdcolin merged commit 36fb12a into main Feb 9, 2025
4 checks passed
@cmdcolin cmdcolin deleted the job_list_name_as_id branch February 9, 2025 02:46
@cmdcolin
Copy link
Collaborator

cmdcolin commented Feb 9, 2025

lgtm. another case of "name" being an id, but such is life :)

@cmdcolin
Copy link
Collaborator

cmdcolin commented Feb 9, 2025

possible follow up: it might be better to not silently return the "existing" when adding a job that already exists.

that could be somewhat misleading

@cmdcolin cmdcolin added the enhancement New feature or request label Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants