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

Write tutorial to deploy simple Django app on Heroku #365

Conversation

zkan
Copy link

@zkan zkan commented Oct 7, 2017

Changes proposed in this pull request:

  • Write a tutorial to deploy a simple Hello Email Django app on Heroku.

Connected to #356.

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 7, 2017
@SendGridDX
Copy link

SendGridDX commented Oct 7, 2017

CLA assistant check
All committers have signed the CLA.

@zkan
Copy link
Author

zkan commented Oct 17, 2017

@thinkingserious Could you please review this PR? I'd love to see some feedback. :)

USE_CASES.md Outdated
Now we should be able to send an email. Let's run our Django development server to test it. Find the file `manage.py` then run:

```
$ python manage.py runserver
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we tell them to:
cd hello_sendgrid
?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, when I ran this, I got this error:

You have 13 unapplied migration(s). Your project may not work properly until you apply the migrations for app(s): admin, auth, contenttypes, sessions.
Run 'python manage.py migrate' to apply them.

I ran python manage.py migrate and then the server started running properly

apikey=os.environ.get('SENDGRID_API_KEY')
)
from_email = Email('[email protected]')
to_email = Email('[email protected]')
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we suggest they put their own email in the to_email variable, so that they can see the email they receive?

Copy link
Author

Choose a reason for hiding this comment

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

I've added a note below this code already.

Copy link
Contributor

@mbernier mbernier left a comment

Choose a reason for hiding this comment

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

I have some suggestions as well as a problem deploying to heroku. I really appreciate your help!

USE_CASES.md Outdated
Now we should be able to send an email. Let's run our Django development server to test it. Find the file `manage.py` then run:

```
$ python manage.py runserver
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, when I ran this, I got this error:

You have 13 unapplied migration(s). Your project may not work properly until you apply the migrations for app(s): admin, auth, contenttypes, sessions.
Run 'python manage.py migrate' to apply them.

I ran python manage.py migrate and then the server started running properly

USE_CASES.md Outdated
$ python manage.py runserver
```

By default, it starts the development server at `http://127.0.0.1:8000/`. To test if we can send email or not, go to `http://127.0.0.1:8000/sendgrid/`. If it works, we should see the page says "Email Sent!".
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add something to the effect:

Note: If you use [email protected] as your from email, it's likely to go to your spam folder. To have the emails show up in your inbox, try using an email address at the domain you registered your SendGrid account.

from .views import index


urlpatterns = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a test app, how do you feel about using the following URL patterns to make things a bit easier?

urlpatterns = [
    url(r'^admin/', admin.site.urls),
    url(r'^$', index, name='sendgrid'),
    url(r'', index, name='sendgrid'),
]

Then we can add a note to say "These paths allow the root URL to send the email, for a true django app, you may want to move this script to another url, like so:

urlpatterns = [
     url(r'^admin/', admin.site.urls),
     url(r'^sendgrid/', index, name='sendgrid'),
 ]

Copy link
Author

Choose a reason for hiding this comment

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

I've revised my text and kept only one url like this:

urlpatterns = [
    url(r'^admin/', admin.site.urls),
    url(r'^$', index, name='sendgrid'),
]

I think having one URL for index should be enough. What do you think?

USE_CASES.md Outdated
@@ -171,6 +172,186 @@ print(response.body)
print(response.headers)
```

<a name="hello_email_django_on_heroku"></a>
# Deploy a Simple Hello Email Django App on Heroku
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this to "Create a Django app to send email with SendGrid"

-- This way, it's a top level section about Django and we can add other deployment methods as H2s below.

USE_CASES.md Outdated
@@ -4,6 +4,7 @@ This documentation provides examples for specific use cases. Please [open an iss

* [Transactional Templates](#transactional_templates)
* [Attachment](#attachment)
* [Deploy a Simple Hello Email Django App on Heroku](#hello_email_django_on_heroku)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add:

* [Create a Django app to send email with SendGrid](#create-a-django-app-to-send-email-with-sendgrid)
  * [Deploy to Heroku](#deploy-to-heroku)

USE_CASES.md Outdated

We also need to do a couple things:

1. Add `'*'` or your Heroku app domain to `ALLOWED_HOSTS` in the file `settings.py`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Will look like

ALLOWED_HOSTS = ['*']

USE_CASES.md Outdated

## Deploy to Heroku

Before we start the deployment, let's log in to your Heroku account and create a Heroku app. This tutorial uses `hello-sendgrid`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of logging in to create the app, can we suggest taht just after git init below, that the user do this:

heroku create hello-sendgrid

Note: if they see heroku reply with "this already exists" suggest adding a random string to the end of the name. They will need to update this line with the new name:
heroku git:remote -a hello-sendgrid

```
$ git add .
$ git commit -am "Create simple Hello Email Django app using SendGrid"
$ git push heroku master
Copy link
Contributor

Choose a reason for hiding this comment

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

I got to here and then got this error, I tried creating a runtime.txt file and that didn't change anything.

remote: Compressing source files... done.
remote: Building source:
remote:
remote:  !     No default language could be detected for this app.
remote:                         HINT: This occurs when Heroku cannot detect the buildpack to use for this application automatically.
remote:                         See https://devcenter.heroku.com/articles/buildpacks
remote:
remote:  !     Push failed
remote: Verifying deploy...
remote:
remote: !       Push rejected to hello-sendgrid-django-python.
remote:
To https://git.heroku.com/hello-sendgrid-django-python.git
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'https://git.heroku.com/hello-sendgrid-django-python.git'

Copy link
Author

Choose a reason for hiding this comment

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

I found a bug in my tutorial. Previously, I said:

$ pip freeze > requirements.text

The issue was Heroku couldn't detect requirements.text. It actually should be requirements.txt. I have fixed that already.

USE_CASES.md Outdated
$ git push heroku master
```

We have not finished yet. We need to go to the Heroku settings and add your `SENDGRID_API_KEY` as one of the Heroku environment variables in the Config Variables section.
Copy link
Contributor

Choose a reason for hiding this comment

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

USE_CASES.md Outdated

We have not finished yet. We need to go to the Heroku settings and add your `SENDGRID_API_KEY` as one of the Heroku environment variables in the Config Variables section.

After that, let's verify if our app is working or not by accessing the Heroku app domain and going to `/sendgrid/`. You should see the page says "Email Sent!" and on the Activity Feed page in the SendGrid dashboard, you should see a new feed with the email `[email protected]`.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use the routing I added above, they can go to the root of the heroku app

@mbernier mbernier added difficulty: hard fix is hard in difficulty hacktoberfest labels Oct 22, 2017
@codecov-io
Copy link

codecov-io commented Oct 24, 2017

Codecov Report

Merging #365 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #365   +/-   ##
=====================================
  Coverage      83%    83%           
=====================================
  Files          30     30           
  Lines        1024   1024           
  Branches      160    160           
=====================================
  Hits          850    850           
  Misses         84     84           
  Partials       90     90

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ec799f...50ea2f4. Read the comment docs.

@zkan
Copy link
Author

zkan commented Oct 24, 2017

@mbernier Thank you very much for your feedback. I've revised my tutorial already.

@mbernier
Copy link
Contributor

mbernier commented Nov 3, 2017

@zkan

We have not been able to merge your Pull Request, but because you are awesome - we wanted to make sure you could still get a SendGrid Hacktoberfest shirt.

Please go fill out our swag form before Nov 5th and we will send the shirt! (We know that you might have tried this before and it didn’t work, sorry about that!)

You have till Nov 5th to fill out this form in order to get the Hacktoberfest shirt!

Thank you for contributing during Hacktoberfest! We hope to see you in the repos soon! Just so you know, we always give away a SendGrid shirt for your first ever non-Hacktoberfest PR that gets merged.

@zkan
Copy link
Author

zkan commented Nov 4, 2017

@mbernier Thank you! I've just filled out the form.

I'm also very happy to continue working on this PR until it gets merged. :)

@thinkingserious
Copy link
Contributor

Thanks @zkan!

USE_CASES.md Outdated

urlpatterns = [
url(r'^admin/', admin.site.urls),
url(r'^$', index, name='sendgrid'),
Copy link
Contributor

Choose a reason for hiding this comment

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

With this code, I get an error when trying to go to http://127.0.0.1:8000/sendgrid/: https://screencast.com/t/NrcQ2pHAtWi

But if I use url(r'^sendgrid/', index, name='sendgrid') it works fine.

Copy link
Author

Choose a reason for hiding this comment

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

@thinkingserious Thank you for the review! I've revised the text and removed the part I think it may confuse readers.

- Remove the root URL setup part that may confuse readers
- Revise the text
@thinkingserious thinkingserious merged commit 04fb74f into sendgrid:master Dec 1, 2017
@thinkingserious
Copy link
Contributor

Hello @zkan,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

@zkan zkan deleted the write_tutorial_to_deploy_django_to_heroku branch December 3, 2017 00:10
@zkan
Copy link
Author

zkan commented Dec 3, 2017

@thinkingserious @mbernier Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: hard fix is hard in difficulty status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants