Skip to content

Conversation

nehzata
Copy link
Contributor

@nehzata nehzata commented Nov 9, 2023

@alecthomas What's your position on this change? I've come across a case where an Options Handler needs access to information contained in the url params.

eg:
//happy:api GET /v1/metrics/:orgId authenticated permissioned

In this case the permissioned option handler needs access to :orgId in the URL to determine if the authenticated user has access to that org.

This PR changes the []string of params to a map[string]string and merges it's content with the options map. I feel it's a little hacky but to fix it properly required substantial more work. Next stage I think we should factor out the route, params & body parsing section out as it's kind of duplicated in ServeHTTP and HandlerOptions..

@nehzata nehzata requested a review from alecthomas November 9, 2023 00:54
@alecthomas
Copy link
Collaborator

Seems reasonable to me!

@nehzata nehzata merged commit 12a6ee0 into master Nov 9, 2023
@nehzata nehzata deleted the include-params-in-options branch November 9, 2023 02:07
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