-
Notifications
You must be signed in to change notification settings - Fork 117
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
Group endpoints #360
base: main
Are you sure you want to change the base?
Group endpoints #360
Conversation
Reading through the code this looks great. Thanks, @dklmuc! I would like to pull it down locally and play with it a bit before merging though, so it will have to wait until I have some time to do that. Really appreciate the effort! |
no problem, we have our customized version (which I'm trying to bring back to an updateable state) ;-) Also already thought about making this configurable per setting, so I'm open to change requests. |
I could see it being somewhat annoying if there's only 1 thing in every "group". Maybe this should only kick in after there are > N resources? I don't know what that number should be. |
Imho I think a boolean setting would be best if someone didn't want to use it (or make it default false). We have got an API here with 17 first level "groups", some of them with up to 30 endpoints and some of them with only 3 or less endpoints. Which looks great also when some of the small "groups" coming one after another. I don't think a mixture depending on the count would add benefits here. |
I wasn't proposing a mixture, just avoiding the boolean config by turning it on once there are more than >N total resources. (It would be a bit overkill if your API has 2-3 resources) |
I think in the end somebody will ask for lowering the defined value, then somebody will come and ask to raise it again. So finally it will become a configurable setting to group on higher ressource counts. Personally I would set it to zero, also for small APIs, therefor I've thought about just on/off. |
One of the values I tried to bake into Taffy was that configuration is evil. You can't get completely away from it, but in most cases simply letting the framework "have an opinion" is enough to eliminate proposed config. I think this is a good addition and if we want it to be zero-config (which I do) then I think always-on is the right way to go. |
Any idea if or when this will be merged to master? I'd love to use this feature with our current REST api and would love it if it's in master as is |
I'd love to add it -- I manage an API with several dozen resources too and this would be a great addition. I still think we want to add a config setting to either enable/disable this behavior, or set the threshold for minimum total # of resources before it turns on. I don't have a strong preference for one or the other of those implementations, but whichever we go with I'd like to see it default to turned on (so if it's the minimum total, default to 0). |
@dklmuc my preference would be to always include the full URI. /messages |
Checking in @dklmuc. I'd still like to get this landed. |
This seems like a great feature. @atuttle : If @dklmuc doesn't have the bandwidth right now, I can take a look at it. Can you let me know which part(s) from discussion above still need to be worked on? 1. the zero-config aspect with n>X triggering it and/or 2. the specific output of the groupnames / subitem URIs? Thanks. |
Relates to #299 (comments)
Group endpoints in dashboard and docs for better clarity