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

fix: Deduplicate tags before sending to Kong #183

Merged
merged 1 commit into from
Jul 6, 2020
Merged

Conversation

cwurm
Copy link
Contributor

@cwurm cwurm commented Jul 4, 2020

When getting entities from Kong, decK will restrict the query to the tags in the state files. However, when using multiple state files, it will AND all tags it finds together - regardless of whether they are unique tags or just the same tags.

When the total number of tags is above 5, Kong will reject the query outright.

This PR deduplicates tags before they are sent to Kong. This allows having the same tag(s) in every file (to make sure it is never synced tag-less) - as long as the total unique number is not above 5.

Copy link
Member

@hbagdi hbagdi left a comment

Choose a reason for hiding this comment

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

I'd rather see existing := map[string]bool instead of struct{} but that's not a blocker.

@hbagdi
Copy link
Member

hbagdi commented Jul 6, 2020

Although decK merges the tags from all files, that was not what I had intended. I was aware of the behavior but I was too lazy to fix it, which has now come back to bite me. Another lesson that limit the room for interpretation to the minimum in a user facing API. I've learned this lesson so many times but it just doesn't stick in my head. (:rage:, Harry)

Thanks for the fix here @cwurm!

@hbagdi hbagdi merged commit 412482e into Kong:master Jul 6, 2020
@cwurm cwurm deleted the dedup_tags branch July 7, 2020 10:49
rainest pushed a commit that referenced this pull request Apr 21, 2021
AntoineJac pushed a commit that referenced this pull request Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants