-
Notifications
You must be signed in to change notification settings - Fork 930
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
Entity type refactor (simplified) #13846
Entity type refactor (simplified) #13846
Conversation
a232935
to
fa6bfa2
Compare
@markylaing can we mark this one as "closes #13262"? and "closes #12928"? |
fa6bfa2
to
6946912
Compare
@tomponline @markylaing Entities will need to be grouped to classify endpoints for the API metrics, grouping them on the metrics package makes sense but I think it would fall out of date eventually because one would have to update the metrics package as weel when adding an entity type that did not fall within the previous categories. Do you think it would be appropriate to extend this implementation in the futute with a field on The groups themselves would also be created within the entity package Edit: This comment can just be disregarded as we agreed upon just using the approach outlined in the PR description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, a couple of nits then ready to go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as the metrics go, this look good. Thanks for this, will help a lot!
Signed-off-by: Mark Laing <[email protected]>
6946912
to
e1298b9
Compare
Signed-off-by: Mark Laing <[email protected]>
Signed-off-by: Mark Laing <[email protected]>
e1298b9
to
fa09b2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@tomponline @hamistao this is a much more concise entity type refactor that I think works much better. (Thanks Tom for the idea of keeping the types as they are, but storing them in a map the returns a struct/interface with relevant fields/methods).
With this change, I think it should be easy to add the URL prefix classification by:
PrefixMatch() []string
method totypeImpl
that returns a slice of URL prefixes to match the entity type against.MatchURLPrefix
which iterates over theentityType
map, and iterates over thePrefixMatch()
array for eachtypeImpl
, then returns the entity type ifstrings.HasPrefix(u.Path, prefix)
.Closes #13262
Closes #12928