Skip to content

Add column external_uuid to contact/contactgroup table #216

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
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add column external_uuid to contact/contactgroup table #216

wants to merge 3 commits into from

Conversation

@sukhwinder33445
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 commented Jun 11, 2024 •
edited by Jan-Schuppik
Loading

@sukhwinder33445 sukhwinder33445 self-assigned this Jun 11, 2024
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Jun 11, 2024
@sukhwinder33445 sukhwinder33445 requested a review from nilmerg June 11, 2024 08:29
julianbrost
julianbrost requested changes Jun 11, 2024
View reviewed changes
@sukhwinder33445 sukhwinder33445 force-pushed the add-column-external-uuid branch from 767820c to 0a3cc97 Compare June 11, 2024 09:53
julianbrost
julianbrost requested changes Jun 11, 2024
View reviewed changes
Copy link
Collaborator

@julianbrost julianbrost left a comment

Choose a reason for hiding this comment

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

You're missing the change in the full schema.sql file (looks like I've totally missed this before).

Also, if the NOT NULL constraints remain,

  1. a value should be set for existing rows.
  2. this should not be merged without Icinga/icinga-notifications-web#199, otherwise this would break creating contacts.

@sukhwinder33445 sukhwinder33445 force-pushed the add-column-external-uuid branch 5 times, most recently from b997f95 to bcc0bf0 Compare June 12, 2024 12:31
@sukhwinder33445 sukhwinder33445 mentioned this pull request Jun 12, 2024
Closed
raviks789
raviks789 reviewed Jun 12, 2024
View reviewed changes
@sukhwinder33445 sukhwinder33445 force-pushed the add-column-external-uuid branch from bcc0bf0 to 2813f1e Compare June 13, 2024 07:34
@sukhwinder33445 sukhwinder33445 force-pushed the add-column-external-uuid branch from 2813f1e to c630f50 Compare June 27, 2024 13:23
julianbrost
julianbrost reviewed Jun 27, 2024
View reviewed changes
Copy link
Collaborator

@julianbrost julianbrost left a comment

Choose a reason for hiding this comment

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

General question: do you (= team web) expect/want this to be merged soon?

schema/pgsql/schema.sql Outdated
Comment on lines 60 to 65
CONSTRAINT pk_contact PRIMARY KEY (id),
UNIQUE (username)
UNIQUE (username),
UNIQUE (external_uuid),
CONSTRAINT pk_contact PRIMARY KEY (id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering why you reordered the constraints here. Also, in the rest of the schema file, so far the primary key is given first, I'd keep it that way (it also makes sense to me, one can say it's the most important constraint).

@sukhwinder33445 sukhwinder33445 force-pushed the add-column-external-uuid branch from c630f50 to ebe647d Compare June 27, 2024 19:49
@sukhwinder33445
Copy link
Contributor Author

This can wait, so far it is a stand-alone change without any dependencies to other branches (except the linked web branch). When the corresponding web branch is ready to be merged, this should be merged together with it.

@nilmerg nilmerg force-pushed the add-column-external-uuid branch from ebe647d to 8f8e51c Compare August 26, 2025 09:15
@nilmerg
Copy link
Member

nilmerg commented Aug 26, 2025

I've only rebased and fixed the conflicts.

@Jan-Schuppik Jan-Schuppik mentioned this pull request Sep 3, 2025
Open
@nilmerg
pgsql: Conform to current schema definition
80a6186
@nilmerg
mysql: Introduce external_uuid to contact, contactgroup and `ch…
ed33127 
…annel`

The `UUID()` function exists since MySQL 8.0 and is available in
MariaDB 10.2.2. Both current minimum requirements for Icinga DB.
@nilmerg nilmerg force-pushed the add-column-external-uuid branch from eb223eb to ed33127 Compare October 22, 2025 12:06
@nilmerg nilmerg requested a review from julianbrost October 22, 2025 12:10
@nilmerg
Copy link
Member

nilmerg commented Oct 22, 2025

Added mysql changes. Should be somewhat final now.

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

Reviewers

@raviks789 raviks789 raviks789 left review comments

@nilmerg nilmerg Awaiting requested review from nilmerg

@julianbrost julianbrost Awaiting requested review from julianbrost

Requested changes must be addressed to merge this pull request.

Assignees

@sukhwinder33445 sukhwinder33445

Labels

cla/signed CLA is signed by all contributors of a PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add column external_uuid to contact and contactgroup

4 participants

@sukhwinder33445 @nilmerg @julianbrost @raviks789