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

migrate_predefined_labels can create duplicate dynamic grouped labels #400

Open
joshelser opened this issue Oct 3, 2023 · 1 comment
Open
Assignees
Milestone

Comments

@joshelser
Copy link
Contributor

When running finalize_setup() when the homepage loads, I've noticed I will sometimes get duplicate column name: 'dbt Models' and there will be multiple rows for the predefined dynamic grouped labels.

I believe the MERGE condition is wrong as it is looking only at name which is NULL for all dynamic grouped labels (rather than consider name+group_name)

@joshelser joshelser self-assigned this Oct 3, 2023
@joshelser joshelser added this to the Sprint 26 milestone Oct 3, 2023
joshelser added a commit to joshelser/OpsCenter that referenced this issue Oct 3, 2023
Because the MERGE only inspected the label name (which is null for
predefined grouped labels), we were incorrectly falling into the not
matched condition and inserting duplicate rows.

Closes sundeck-io#400
@joshelser
Copy link
Contributor Author

Need to fix this the correct way, else we'll keep having more.

  • dynamic labels need to use the name column and not the group_name column
  • the UI needs to be updated to detect grouped labels from the IS_DYNAMIC column
  • SQL procs need to be updated to undo the special handling of group_name instead of name (in the crud layer)
  • Finally, make sure the reported problems here and in Predefined dynamic grouped labels don't get recreated after reset #402 are then addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

1 participant