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.

Sign up for GitHub

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

Jump to bottom

Bump flask jwt extended version to 4.0.2 #1002 #1005

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

Bump flask jwt extended version to 4.0.2 #1002 #1005

wants to merge 3 commits into from

Conversation

epicadk
Copy link
Member

@epicadk epicadk commented Feb 17, 2021 •
edited by isabelcosta
Loading

Description

Bump flask jwt extended version to 4.0.2
Fixes #1002

Type of Change:

  • Code

Code/Quality Assurance Only

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

How Has This Been Tested?

Ran existing unit tests

Checklist:

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

Code/Quality Assurance Only

  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been published in downstream modules

@codecov
Copy link

codecov bot commented Feb 17, 2021 •
edited
Loading

Codecov Report

Merging #1005 (f7bc61a) into develop (c154317) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1005   +/-   ##
========================================
  Coverage    93.25%   93.25%           
========================================
  Files           38       38           
  Lines         2062     2062           
========================================
  Hits          1923     1923           
  Misses         139      139
Impacted Files Coverage Δ
app/api/jwt_extension.py 100.00% <100.00%> (ø)
app/api/resources/admin.py 88.70% <100.00%> (ø)
app/api/resources/mentorship_relation.py 97.07% <100.00%> (ø)
app/api/resources/task.py 100.00% <100.00%> (ø)
app/api/resources/task_comment.py 100.00% <100.00%> (ø)
app/api/resources/user.py 90.36% <100.00%> (ø)

@epicadk epicadk added the Status: Needs Review PR needs an additional review or a maintainer's review. label Feb 17, 2021
devkapilbansal
devkapilbansal previously approved these changes Feb 18, 2021
View reviewed changes
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.

LGTM 👍

isabelcosta
isabelcosta previously approved these changes Apr 2, 2021
View reviewed changes
Copy link
Member

@isabelcosta isabelcosta left a comment

Choose a reason for hiding this comment

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

Thank you for working on this @epicadk !
This looks good!

@isabelcosta isabelcosta added Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Apr 2, 2021
@isabelcosta isabelcosta temporarily deployed to ms-backend-review-pr-1005 April 2, 2021 12:30 Inactive
@isabelcosta
Copy link
Member

Not sure why, but when I tried to deploy this to Heroku Review App is giving 500 Internal Server Error :O
https://ms-backend-review-pr-1005.herokuapp.com/ cc @epicadk @devkapilbansal
I have no idea why 🤔 and why tests pass and this fails

@devkapilbansal
Copy link
Member

devkapilbansal commented Apr 2, 2021 •
edited
Loading

@isabelcosta may I know these environment variables on heroku

DB_TYPE
DB_USERNAME
DB_PASSWORD
DB_ENDPOINT
DB_NAME

@devkapilbansal
Copy link
Member

P.S.:- Changing DB_TYPE to postgresql should solve the issue

@isabelcosta
Copy link
Member

isabelcosta commented Apr 2, 2021 •
edited
Loading

@isabelcosta may I know these environment variables on heroku

DB_TYPE
DB_USERNAME
DB_PASSWORD
DB_ENDPOINT
DB_NAME

Yes, it's something like this:

DB_TYPE='postgres'
DB_USERNAME='XXXXX'
DB_PASSWORD='XXXXXXXXXXXXXXXXXXXXXXX'
DB_ENDPOINT='drona.db.elephantsql.com:5432'
DB_NAME='qieoopii'

@devkapilbansal same as the deployed server db

@epicadk
Copy link
Member Author

epicadk commented Apr 2, 2021 •
edited
Loading

Looking into this. It's working fine for me. But I did have to run pip install -Ur requirements.txt

@devkapilbansal
Copy link
Member

Looking into this. It's working fine for me. But I did have to run pip install -Ur requirements.txt

Probably we need to update some other dependencies too

@devkapilbansal
Copy link
Member

Adding it to TODO. Will search for the root cause soon

@epicadk
Copy link
Member Author

epicadk commented Apr 7, 2021

@isabelcosta can you share the log ?

@devkapilbansal devkapilbansal removed their assignment Apr 13, 2021
@devkapilbansal devkapilbansal mentioned this pull request May 16, 2021
Closed
@devkapilbansal
Copy link
Member

@isabelcosta can you please test it again

vj-codes
vj-codes requested changes Jul 24, 2021
View reviewed changes
Copy link
Member

@vj-codes vj-codes left a comment

Choose a reason for hiding this comment

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

@epicadk please resolve the merge conflicts

@vj-codes vj-codes added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. labels Jul 24, 2021
@epicadk epicadk dismissed stale reviews from isabelcosta and devkapilbansal via 9fdde9f July 26, 2021 06:03
devkapilbansal
devkapilbansal approved these changes Jul 26, 2021
View reviewed changes
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.

LGTM 👍

@isabelcosta isabelcosta temporarily deployed to ms-backend-review-pr-1005 July 29, 2021 22:31 Inactive
@isabelcosta
Copy link
Member

:( I keep seeing the 500 error

internal server error

I just used a review app from another PR, and it worked fine. Also on heroku, I can't seem to understand how to check the logs for Review Apps. :(

I will check this later again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vj-codes vj-codes vj-codes requested changes

@devkapilbansal devkapilbansal devkapilbansal approved these changes

@isabelcosta isabelcosta isabelcosta left review comments

@gaurivn gaurivn Awaiting requested review from gaurivn

Requested changes must be addressed to merge this pull request.

Assignees

@epicadk epicadk

Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bump Flask jwt extended version to 4.0.2
4 participants
@epicadk @isabelcosta @devkapilbansal @vj-codes