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

[Feat]: Add heroku configuration for deployment #103

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

Rahulm2310
Copy link
Contributor

Description

Add heroku configuration for deployment
Fixes #80

Instructions for deployment :

  1. Login to Heroku
    heroku login
    
  2. After you have executed the command ‘heroku login’, in the web browser, you are now redirected to the page as shown in the image below. Here, you are required to click on the ‘Log In’ button.
  3. Now, you are required to create an app in Heroku. Hence, for the stated purpose you are required to execute the following command.
    heroku create open-source-programs-backend
    
  4. After the successful execution of the command you can now view your application in Heroku devcenter.
  5. For deploying your application you are required to execute the following command.
    git push heroku develop
    
  6. To commit initial migration on the Heroku application you are required to execute the following query.
    heroku run python manage.py migrate
    
  7. Finally, you can execute the following command to view the hosted website.
    heroku open
    

Type of Change:

  • Code
  • Outreach

Code/Quality Assurance Only

  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Not Applicable

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

Code/Quality Assurance Only

  • My changes generate no new warnings

@Rahulm2310
Copy link
Contributor Author

@codesankalp @isabelcosta Can you please review this PR. Thanks 🙂

@codesankalp
Copy link
Member

@Rahulm2310 Will this be able to deploy from the Heroku pipeline through github?

QA checks are also failing.

main/settings.py Outdated
@@ -28,9 +29,9 @@
SECRET_KEY = "7ibm!g0j@gs7kszwav!6$*lar(+!!l3tpm@09s%csl2bj)l+p4"

# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True
DEBUG = False
Copy link
Member

Choose a reason for hiding this comment

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

For deployment take this DEBUG value from environment variables else for development make this True.

main/settings.py Outdated Show resolved Hide resolved
main/settings.py Outdated Show resolved Hide resolved
@codesankalp codesankalp added the Status: Needs Review PR needs an additional review or a maintainer's review. label Feb 28, 2021
ProcFile Outdated Show resolved Hide resolved
@codesankalp
Copy link
Member

@Rahulm2310 Follow the README guidelines for QA-Checks as both workflows are failing.

main/settings.py Outdated
@@ -155,3 +156,9 @@
# https://docs.djangoproject.com/en/3.0/howto/static-files/

STATIC_URL = "/static/"

STATIC_ROOT = os.path.join(BASE_DIR, ‘static’)
Copy link
Member

Choose a reason for hiding this comment

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

Please don't include backticks use double-quoted strings.

@Rahulm2310
Copy link
Contributor Author

@codesankalp Can you please explain why both workflows are still failing?

@codesankalp
Copy link
Member

@Rahulm2310 Try to follow the commands mentioned in README - https://github.com/anitab-org/open-source-programs-backend#qa-checks
You can also check the suggestion of the workflow.
When QA-Checks will pass I will review this PR.

Copy link
Member

@codesankalp codesankalp left a comment

Choose a reason for hiding this comment

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

@Rahulm2310
I tried it on the local system and it is returning 500 server errors for URL which is working.
Please look into that.
image

Also, You have updated the requirements.txt wrong. Either use the virtual-env to update packages or add dependencies separately. As currently there are 54 dependencies including flask and other stuff which is not needed I think.

@codesankalp codesankalp added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Mar 7, 2021
@codesankalp
Copy link
Member

For testing, you can use gunicorn main.wsgi.

@Rahulm2310
Copy link
Contributor Author

I tried it on the local system and it is returning 500 server errors for URL which is working.

I think that is may be because you don't have DJANGO_DEBUG in your .env file.
Also, I am getting this when running gunicorn main.wsgi.
image

@codesankalp
Copy link
Member

codesankalp commented Mar 8, 2021

@Rahulm2310 I have DJANGO_DEBUG in my .env file. DJANGO_DEBUG=False will be used in production which only means any error will not be shown on the deployed backend.
And I am not sure why gunicorn main.wsgi returns an error in windows because I tried it on Ubuntu. You can research about this error.
But what I wanted is that after running this command it must not show any errors (Server error 500).

@Rahulm2310
Copy link
Contributor Author

@codesankalp I came to know that gunicorn is for unix environment and is incompatible with windows. There is an open issue on the gunicorn's github repo for this.

@codesankalp
Copy link
Member

Then @Rahulm2310 there is only one option try deploying on Heroku for testing your code.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 8, 2021

@codesankalp is this the correct way DEBUG = os.environ.get("DJANGO_DEBUG", "") != "False" ?
Also, this works if I remove DJANGO_DEBUG from .env file.

@codesankalp
Copy link
Member

@Rahulm2310 The line that you have added is correct i.e. it will make DEBUG=False when DJANGO_DEBUG=False will be present in .env file else it will make DEBUG=True in any other remaining condition.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 8, 2021

@codesankalp I tried deploying to heroku. But getting this error when running heroku run python manage.py migrate.
image.
image

Although, I have download file in the project's root still shows this.

@codesankalp
Copy link
Member

codesankalp commented Mar 9, 2021

Try to rebase it with the current develop branch then there will be no errors regarding Zulip Key.
The download file is in your local directory but as it is also present in .gitignore because of this it is not present in heroku.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 9, 2021

@codesankalp I rebased with current develop, pushed the download file to heroku and then heroku run python manage.py migrate ran successfully. Here's the deployed link https://open-source-programs-backend.herokuapp.com/api. Could you please check and confirm if it looks alright so that I can push the changes here.

@codesankalp
Copy link
Member

image
I tried making a user and it returns a server error.
Maybe you need to set up all environment variables (including sendgrid email) in the .env file.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 11, 2021

Maybe you need to set up all environment variables (including sendgrid email) in the .env file.

I tried adding SENDGRID_API_KEY to heroku config vars(environment variables), still showing the same error. Also, when I tried it locally it gives me this error
image
Although, my from address is verified sender identity on sendgrid, still getting this error.

@codesankalp
Copy link
Member

Maybe you need to set up all environment variables (including sendgrid email) in the .env file.

I tried adding SENDGRID_API_KEY to heroku config vars(environment variables), still showing the same error. Also, when I tried it locally it gives me this error
image
Although, my from address is verified sender identity on sendgrid, still getting this error.

Is it working on local and not working on heroku?

@Rahulm2310
Copy link
Contributor Author

@codesankalp getting server error when registering a user on heroku.

This is the error when registering a user on local.
image

@codesankalp
Copy link
Member

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 13, 2021

@codesankalp I have my sender email verified on sendgrid.
osp-sendgrid

Can I add you as a collaborator to the deployed project on heroku.

@codesankalp
Copy link
Member

No, It is not needed @Rahulm2310.
If it's a problem related to SendGrid then I will suggest using another email service or consoleBackend.
Let me know if it was able to solve the problem.
Also, Can you access the admin site and was able to create a user from there?

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 15, 2021

If it's a problem related to SendGrid then I will suggest using another email service or consoleBackend.

@codesankalp I tried consoleBackend, and I am getting the verification mail in the console.
osp-console
Seems that this is a problem related to sendgrid.

Also, Can you access the admin site and was able to create a user from there?

yeah I created a super user for heroku app and then created a new user from the admin site.
image

@codesankalp
Copy link
Member

@Rahulm2310 In that case I can approve this PR.
I will review it again soon.

Copy link
Member

@devkapilbansal devkapilbansal left a comment

Choose a reason for hiding this comment

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

@Rahulm2310 CI builds are failing. Please look into them.
Also, your branch is not up-to-date with our default branch. Please rebase your branch.
Look into these requests too:- 👇

ProcFile Outdated
@@ -0,0 +1 @@
web: gunicorn main.wsgi
Copy link
Member

Choose a reason for hiding this comment

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

Needs a newline at the end

README.md Outdated
@@ -95,8 +95,11 @@ Follow the given instructions for Login into the app.
6. Click Create & View.
7. The API KEY is generated and displayed to you just once. So be sure to copy and save it somewhere.

3. `DJANGO_DEBUG` - Set its value to `False` for deployment (not needed for development).
Copy link
Member

Choose a reason for hiding this comment

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

I would rather leave it as DEBUG. Simpler, the better

main/settings.py Outdated

ALLOWED_HOSTS = []
ALLOWED_HOSTS = ["https://open-source-programs-backend.herokuapp.com"]
Copy link
Member

Choose a reason for hiding this comment

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

No need to add https in allowed host

Suggested change
ALLOWED_HOSTS = ["https://open-source-programs-backend.herokuapp.com"]
ALLOWED_HOSTS = ["open-source-programs-backend.herokuapp.com"]

Copy link
Member

Choose a reason for hiding this comment

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

Better to change the name to anitab-forms-backend

main/settings.py Outdated
STATIC_ROOT = os.path.join(BASE_DIR, ‘static’)

# Activate Django-Heroku.
django_heroku.settings(locals())
Copy link
Member

@devkapilbansal devkapilbansal Mar 16, 2021

Choose a reason for hiding this comment

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

You can try something like this

Suggested change
django_heroku.settings(locals())
from socket import gethostname
HOSTNAME = gethostname()
if HOSTNAME == 'open-source-programs-backend.herokuapp.com':
import django_heroku
django_heroku.settings(locals())

Copy link
Member

Choose a reason for hiding this comment

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

@devkapilbansal Why use this when django_heroku doesn't affect the development environment?

Copy link
Member

@devkapilbansal devkapilbansal Mar 16, 2021

Choose a reason for hiding this comment

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

What's the point of setting config if we are using it in development. However, we can ignore this 😕
What's not broken need not to be fixed

@codesankalp
Copy link
Member

@Rahulm2310 Can you please resolve the merge conflict and do the suggested changes?

@Rahulm2310
Copy link
Contributor Author

@codesankalp was a bit more involved in OSH. Will apply the suggested changes asap. 🙂

@Aaishpra
Copy link
Member

Hi, @Rahulm2310 can you do the changes in this, as the deployment of the backend, is one of the high priority things for us currently.

@codesankalp
Copy link
Member

@Rahulm2310 It is a priority now because the deployed backend is required. Can you please update the PR?

@Rahulm2310
Copy link
Contributor Author

@codesankalp can you help me identify why CI builds are failing

@codesankalp
Copy link
Member

@codesankalp can you help me identify why CI builds are failing

Not sure, It is something related to postgres. I think while deleting the database it can't delete it.
I will research about it more.

@@ -0,0 +1,4 @@
[api]
Copy link
Member

Choose a reason for hiding this comment

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

@Rahulm2310 Remove this file from the commit


ALLOWED_HOSTS = []
ALLOWED_HOSTS = ["anitab-forms-backend.herokuapp.com"]
Copy link
Member

Choose a reason for hiding this comment

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

@Rahulm2310 I think this is deployed in your Heroku account. So adding it here wouldn't be a nice logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Update the develop branch for deployment on heroku.
4 participants