Skip to content

Conversation

eugals
Copy link

@eugals eugals commented Oct 17, 2016

The is a relatively new feature described here: In addition to /some/path/{tail} route format there may be /some/path/{tail+} resource declarations in API Gateway now.
And this short patch teaches Chalice how to deal with them.

@jamesls
Copy link
Member

jamesls commented Oct 24, 2016

I'm interested to hear how you'd plan on using this in chalice. I'm not opposed to the change, but I'm not really sure how helpful this is for chalice. Would others find this useful? Feel free to chime in with use cases.

@eugals
Copy link
Author

eugals commented Oct 24, 2016

@jamesls

Here is a quote from the documentation page I gave link to at the beginning:

A special path parameter denoted as {proxy+}. This path parameter represents any of the child resources under its parent resource of an API. In other words, /parent/{proxy+} can stand for any resource matching the path patten of /parent/*. The + symbol indicates to API Gateway to intercept all requests on the matched resource. This special path parameter is also known as a greedy path variable. The proxy variable is the greedy path variable name and can be replaced by another string in the same way you treat a regular path parameter name.

This is just a new and useful API Gateway feature which needs to be supported in Chalice.
In fact it is possible to write something like this already:

@app.route('/objects/{keyPath+}', methods=['GET', 'PUT'])
def myobject(keyPath):
    keyPath = keyPath.split('/')
    # ...

This code would be accepted by the deployer, and the proper API Gateway resource tree would be created successfully in AWS cloud.
But when I try to call this method, an exception would be raised on the lambda side, because of the bug in Chalice I'm proposing a fix to with this simple patch.

@jamesls
Copy link
Member

jamesls commented Oct 24, 2016

Sorry, I don't think I explained what I was looking for very well.

I'm familiar with the feature and what it does, what I'm curious about is how people plan on using it in chalice. The common use case I've seen for this feature is to make integration with pre-existing frameworks. That is, if your framework already handles routing for you, you just need a catch-all greedy route that can be passed to your framework. This feature is perfect for that.

Given chalice doesn't use that, and is using API gateway to handle the routing, I'm trying to envision how people would like to use this functionality in chalice.

I'm trying to keep the feature set small, so I generally ask for each feature that's added to chalice to have a rationale for why it should be included (if it's not clear to me).

If you (or anyone else) could give a few concrete use cases of how this feature would be helpful to you, that would be sufficient for me.

Does that seem reasonable?

@jamesls
Copy link
Member

jamesls commented Oct 25, 2016

Ah you know what, your last example is a good use case (I missed that earlier). If I want to have a view that takes an S3 key as part of the uri, I'd need to use a greedy path. That seems like a pretty reasonable thing to do in chalice.

Thanks for your patience on this. I'm going to just add a few tests for this PR and then merge.

@andriisoldatenko
Copy link
Contributor

@jamesls Do you need help with unittests?

@freaker2k7
Copy link

That'd be also lovely to catch 404 pages, sigh...

@orenbarzilai
Copy link

Any updates regarding the {proxy+} support?

@eugals
Copy link
Author

eugals commented Apr 9, 2017

I don't know why it should take so long to simply merge this very primitive and straightforward bugfix into the main repo.

I myself gave up on this Chalice framework long time ago.
The idea looked promising at first glance, but the implementation is buggy and the feature set is not flexible enough for my purposes. So I ended up switching on Zappa...

@freaker2k7
Copy link

I found the default Zappa performance worse than Chalice's...
Not into tweaking yet... :)

But the greedy path here would be Awesome !!!

@jamesls
Copy link
Member

jamesls commented May 25, 2017

Hey sorry, just wanted to give an update here. I have no issues with this feature, but I wanted to give a list of things we'll need before this can get merged:

  • unit tests
  • works with chalice local (I think there's some minor changes needed in the routing code there)
  • documentation

If someone would like to take a stab at this let me know. Otherwise I have this on my backlog but I don't have an exact timeframe.

@freaker2k7
Copy link

I got my own code to write.. but as far as I can assist, btw, I started using chalice from version 0.6.0 and stopped upgrading at 0.7.0 after I figured out the missing deployed.json file, because > 0.8.0 stopped serving unicode on my localhost and my pc doesn't have a CloudFront to send the safe base64 to, but back to the issue..

for the route
@app.route('/{a}/{b}/{c}/{proxy+}', methods=['GET'])
def some_route(a, b, c): #...
the "proxy+" argument is in:
app.current_request.uri_params['proxy']

so... I think you should simply document it 👍

@eugals eugals closed this Jul 4, 2017
@kislyuk
Copy link
Contributor

kislyuk commented Aug 9, 2017

@jamesls, any updates on how greedy path variables can be used with chalice?

@russau
Copy link

russau commented Mar 25, 2019

@freaker2k7 is your code sample able to handle a route like /alpha/bravo/charlie/delta/echo? I did a quick test and it looks like the uri_params['proxy'] will only contain delta, i.e. we are not getting a greedy match.

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

Successfully merging this pull request may close these issues.

7 participants