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

Do not allow duplicate tags per monitor? #704

Closed
fdcastel opened this issue Oct 14, 2021 · 6 comments
Closed

Do not allow duplicate tags per monitor? #704

fdcastel opened this issue Oct 14, 2021 · 6 comments
Labels
feature-request Request for new features to be added

Comments

@fdcastel
Copy link

Is it a duplicate question?
No.

Is your feature request related to a problem? Please describe.
Maybe.

Describe the solution you'd like
I was a bit surprised when I saw this:

image

What is the value of t1 on monitor1?

I believe monitor_tag table should have a composite primary key (or an UNIQUE key/index) of (monitor_id, tag_id) blocking this occurrence in the db model.

Describe alternatives you've considered
None.

Additional context
If this is indeed the intended design, please disregard. However, I found the semantics of having one tag with multiple values somewhat strange (or that one tag could be "present" multiple times).

I discovered this while working on a PR for #686.

@fdcastel fdcastel added the feature-request Request for new features to be added label Oct 14, 2021
@louislam
Copy link
Owner

It is by design, more details: #278

Same Tags with different values = OK
Same Tags with same values = Not OK

@fdcastel
Copy link
Author

Ok. 😟 However, I didn't find any rationale for this behavior in #278.

There's a sample scenario? I really can't find any reason for this. It only keeps the code more complex, IMHO.

PR #705 would also be wrong in this case. Given that we cannot uniquely identify a tag just by name, an update of t1=50 will left two t1 tags with value = 50, bringing the monitor to "Not OK" scenario above.

@chakflying
Copy link
Collaborator

in previous suggestions to implement this feature, tags are mostly used for filtering and grouping. So something like having both DNS and DNS: TXT make sense. Your use case did not even cross my mind when I made this, but I totally see that this will be useful as well.

If you think your use case won't be dealing with duplicate tags, I guess we can just update the first tag we find with that name?

@fdcastel
Copy link
Author

Understood. Thanks for the clarification @chakflying .

Yes. Updating the first tag is an option. I will update the PR for this scenario.

@CommanderStorm
Copy link
Collaborator

@fdcastel is this issue still relevant?
If not, could you close it? (otherwise this will become an immortal zombie)

@fdcastel
Copy link
Author

Sure! 🧟 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features to be added
Projects
None yet
Development

No branches or pull requests

4 participants