Adding support for more then just app.py

[joel-petersen]

Adding support for more then just app.py.

The only way I could currently think to make this work was to have an explicit list of files that need to get updated other then the app.py, used to add them to the zip file. Since we're already relying on people having a requirements.txt for pip installation the new file is called internal_requirements.txt .

It is a \n separated list of files to include in the deploy. Supports both files in sub folders and files in the same directory as the app.py.

I didn't create a test for this use case since I wasn't exactly sure about the formatting for them given that this test relies on multiple files. also the current tests before I changed anything didnt succeed for me.

Pasted below is my example files for testing (not included so as to not pollute the project):

./app.py

from chalice import Chalice
from package import fibo
import fibo_2

app = Chalice(app_name='subfolders')
app.debug = True

@app.route('/')
def index():
fibList = fibo.fib2(10)
fibList2 = fibo_2.fib2(10)
return {'hello': 'world', 'fib' : fibList, 'fib2': fibList2 }

./fibo_2.py

Fibonacci numbers module

def fib2(n): # return Fibonacci series up to n
result = []
a, b = 0, 1
while b < n:
result.append(b)
a, b = b, a+b
return result

./package/fibo.py

Fibonacci numbers module

def fib(n): # write Fibonacci series up to n
a, b = 0, 1
while b < n:
print b,
a, b = b, a+b

def fib2(n): # return Fibonacci series up to n
result = []
a, b = 0, 1
while b < n:
result.append(b)
a, b = b, a+b
return result

./package/init.py

./internal_requirements.txt

package
fibo_2.py

And that's it. Any feedback is appreciated and thanks for the easy tool, it was just missing this one feature to really be useful.

[awestendorf]

Confirmed that this works and was very helpful for a test project I'm building today 👍 I don't yet have an opinion on the structure of the solution, but the feature is definitely a requirement.

[dkotik]

Great! It is a bit patchy, but hopefully something similar can be merged into the main branch! Thank you!

[joel-petersen]

Yeah I was kind of on the fence about it. Since the tool already requires a requirements.txt to know what libraries to include, I figured the pattern had already been set to require explicitness.

The alternatives were to just uploading everything in the local directory or requiring everything to be in a subfolder, though both of those are easy alternatives. The negative with that plan is that it would probably require an exclusion file to prevent some stuff from getting uploaded that is undesirable like credentials and less obvious.

[jamesls]

I actually wouldn't be opposed to just supporting a single local directory in which everything gets included. The logic would roughly be:

1. Is there an app/ directory? If so, include everything in app/.
2. Is there an app.py file? If so, just include app.py.
3. Otherwise, error out.

The requirements.txt file is there really because we have to have some way to know what packages and what versions of those packages you want installed. There's not really any "convention" we could follow to automatically get that information.

In terms of the app structure, we're already assuming an implicit app.py, so making that also include an app/ directory would also work.

The plus side would be that everything could still "just work". If we required that app/ was a package with an __init__.py, that means that import app; your_chalice_app = app.app could still work in terms of route generation and policy generation.

What do you all think?

[yeahdef]

I love this idea and can't wait to make use of it.

[joel-petersen]

Just for some context, I implemented this feature before the suggestion of the app directory had been made in another feature request. So when I did implement it, I did it in the most pythonic way I could think of where its generally thought to be better to be explicit instead of implicit (https://www.python.org/dev/peps/pep-0020/), and people's applications wouldn't need to be restructured after merging the pull request.

Now that I've read this alternate proposal, I'm not really against it, but it is going to make two different ways to build apps (and 2 different control paths inside chalice). One which just required an app.py and a separate one where everything needs to be in an app directory and they cant co-exist.

It would mean as soon as you need to split your code into separate files or include non-pip libraries you need to re-structure your app so it was in a sub library.

If the feature is implemented in this more implicit manner, I think it will be clearer and cleaner to just always require an app folder with an app.py in it. And have the chalice new-project structure the application in that way.

One of the advantages of this being beta software is that its possible to require restructuring, without having to always stay backwards compatible.

What do you think?

[jamesls]

You make good points. I hadn't considered that growing an app from a single file to multiple files would unnecessarily require you to move code around. Even in the simplest case of mkdir app; mv app.py app/__init__.py, it still requires you to shuffle code around.

I do like having the flexibility to structure code however you want, and in multiple directories, but I wonder if there's a way to get something more implicit that's based on what people would expect to be reasonable behavior.

As counter-examples I want to avoid, consider the packages kwarg in the setup.py files. In boto, we call out every single package we want to include (https://github.com/boto/boto/blob/develop/setup.py#L62). It's explicit, but at the same time, it seems reasonable that we want to include everything under boto/. It's also something we've forgot to update occasionally, and in newer packages, we just use find_packages (https://github.com/aws/aws-cli/blob/develop/setup.py#L32). Same thing with the MANIFEST.in files. While it's nice to have control over what gets included, it's annoying to always have to call out you do in fact want the README.rst included in the sdist, given that it's common to use long_description=open('README.rst').read() in the setup.py.

Just throwing out other ideas:

a) Any python packages (any top level dir with an __init__.py) and top level modules (*.py) are automatically included.
b) Have a fixed, hard-coded directory that will be included if present. This would be in addition to app.py. So you start with app.py, and if you want to add multiple files, you can create a packages directory and add additional python files within that directory and they'll automatically be included.

Are either of those options any better? Option (b) seems like the best of both worlds.

[dkotik]

this +1:

a) Any python packages (any top level dir with an init.py) and top level modules (*.py) are automatically included.

also aws eb cli pays attention to .git ignore file, could be useful here for flexibility and future-proofing

[kishinmanglani]

@jamesls Maybe a directory named app? I've seen that a lot in flask projects. Otherwise I think option a is also a good solution

[ryansydnor]

I'm personally a fan of option a, which mimics the behavior of find_packages to pull all .py files from folders that contain an __init__.py. It'd keep the packaging of chalice applications consistent with the broader python ecosystem.

[jamesls]

Initial support for multi app files was added in #146. I know there was a good amount of discussion on this. Thanks everyone for the input on this. FWIW, I tried going down the find_packages route, but it didn't have a nice way to include non python files (more info here).

Please give the current version a try and share any feedback you might have. Hopefully it's a reasonable compromise that accommodates most uses cases.