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

Keep track of path when parsing route URL #492

Open
claustres opened this issue Oct 22, 2020 · 8 comments
Open

Keep track of path when parsing route URL #492

claustres opened this issue Oct 22, 2020 · 8 comments

Comments

@claustres
Copy link
Contributor

When parsing the route URL here the path is not kept while it could be useful for some adapters.

For instance, we've developed https://github.com/kalisio/logspout-slack and we need to provide the path part of the webhook (
of the form https://hooks.slack.com/services/xxx) as a configuration option.

@michaelshobbs
Copy link
Member

i'd definitely accept a PR addressing this

@michaelshobbs
Copy link
Member

alternatively, could you use Route.Options to pass the path around?

@claustres
Copy link
Contributor Author

If it seems useful to the community I can try. However, I'm really new to Go and it could take time before I write decent code ;-)

@claustres
Copy link
Contributor Author

@michaelshobbs Do you mean add the path as a query parameter and get it back as an option in the route?

@michaelshobbs
Copy link
Member

Apologies it's been awhile since I've been in this codebase. I assumed you already had a way to get the path into the system and we were just dropping it. Is that true?

@claustres
Copy link
Contributor Author

I am not sure to get you but what I would like to be able to do is for instance:

docker run --name="logspout" \
	--volume=/var/run/docker.sock:/var/run/docker.sock \
	...
	logspout:v3.2.11 \
	slack://hooks.slack.com/services/xxx

But in my adapter when I'd like to get back the route info I can't get the path back:

func NewSlackAdapter(route *router.Route) (router.LogAdapter, error) {
	// Here route.Address is "hooks.slack.com", the path part "/services/xxx" has been lost
        ...
}

Maybe I am wrong but it appears to me neither the type to the parsing take care of it.

Maybe as you suggest there is another way to get the path back as well.

@michaelshobbs
Copy link
Member

Maybe I am wrong but it appears to me neither the type to the parsing take care of it.

that is correct. however it does exist in the variable u (type URL) defined on L91

u, err := url.Parse(expandedRoute)

golang net/url docs: https://golang.org/pkg/net/url/#URL

So we could store u.Path in Route.Options OR we could add another field to the type. I'm fine with either.

@claustres
Copy link
Contributor Author

I think that adding a new field is more in line with the existing base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants