Skip to content

Adds git push template for python webapp2 with google_app_engine sdk #48

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jin09
Copy link

@jin09 jin09 commented Jul 18, 2017

Added git push template for python webapp2. Tested on hasura local dev setup

@coco98
Copy link
Contributor

coco98 commented Jul 18, 2017

@jin09 Thanks for the PR!

I've never used the jetbrains python IDE before, but I don't think we should be committing the .idea directory. If you're sure about this can you point me to a reference that says it's fine?
Here's a reference: https://stackoverflow.com/questions/11968531/what-to-gitignore-from-the-idea-folder

Also, can you include a .gitignore file?

I'll test it out in the meanwhile :)

@jin09
Copy link
Author

jin09 commented Jul 18, 2017

I have now included the .gitignore file and removed .idea folder which was unnecessary

@jin09
Copy link
Author

jin09 commented Jul 26, 2017

Hi,
Did you test it?

Copy link

@ecthiender ecthiender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,

Sorry for the delay in reviewing this. I have added some comments, can you check those out?


RUN pip install -r /home/src/requirements.txt

CMD ["python", "/home/google_appengine/dev_appserver.py", "/home/src/app.yaml", "--skip_sdk_update_check=yes", "--host", "0.0.0.0", "--port", "8080"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can create an app directory instead of putting the source code in /home. That way it will be cleaner.


RUN pip install -r /home/src/requirements.txt

CMD ["python", "/home/src/main.py"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with having an app directory instead of putting it in /home.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, in the directory name of vanilla webapp2 use hyphens instead of underscore (as every other folder is hyphenated). So python-webapp2-vanilla instead of python_webapp2_vanilla.

@@ -0,0 +1,16 @@
import webapp2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requirements.txt is empty. Are you sure it doesn't need webapp2?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it doesn't need webapp2 since it is already installed in the custom base Image that I have built for this (jin09/webapp2)

@jin09
Copy link
Author

jin09 commented Aug 22, 2017

I have made a few changes, please review them.

@coco98
Copy link
Contributor

coco98 commented Aug 22, 2017

@jin09 Sorry for being totally AWOL on this.
I tried this out a while back and forgot to comment. I had a question:
What is the local development workflow for this? Can you add instructions to the README? (Eg: pip install -r requirements.txt followed by python main.py? By local development I mean that I want to test my python app locally before deploying it.

@jin09
Copy link
Author

jin09 commented Aug 23, 2017

@coco98 Workflow for local develoment is in the Dockerfile recipe itself. Or is there anything specific that you want me to add?

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.

3 participants