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

Kk-792 Fixes to the dev environment #237

Merged
merged 4 commits into from
Oct 6, 2021
Merged

Conversation

nikomakela
Copy link
Contributor

@nikomakela nikomakela commented Oct 4, 2021

About the matter: https://stackoverflow.com/questions/33866695/error-installing-psycopg2-on-macos-10-9-5.

Could not upgrade to latest, because it had issues with timezone. This one is also a patch upgrade to last one, so it should not need much testing either.

KK-792
@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2021

Codecov Report

Merging #237 (f03d4f7) into master (1c19fd9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   93.82%   93.82%           
=======================================
  Files         120      120           
  Lines        5506     5506           
  Branches      305      305           
=======================================
  Hits         5166     5166           
  Misses        285      285           
  Partials       55       55           

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 1c19fd9...f03d4f7. Read the comment docs.

@@ -131,14 +131,14 @@ protobuf==3.11.3
# via
# google-api-core
# googleapis-common-protos
psycopg2==2.8.4
psycopg2-binary==2.8.6
Copy link
Contributor

Choose a reason for hiding this comment

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

Using the binary package removes the requirement to have postgres libraries installed for building the psycopg2 package, which is convenient. However psycopg2 documentation states that especially production environments should only use the non-binary version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, nice catch! I'll try to address the problems in some other ways. Using psycopg2-binary was adviced by stack overflow.

Copy link
Contributor Author

@nikomakela nikomakela Oct 6, 2021

Choose a reason for hiding this comment

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

Django 2.2 needs psycopg2 2.8. Mac needs the binary. The psycopg2-binary should be replacesd with psycopg2 when possible: https://www.psycopg.org/docs/install.html#psycopg-vs-psycopg-binary.

So, still using the binary version, but commented in requirements.in that it should be replaced with psycopg2 when possible.

Django 2.2 needs psycopg2 2.8. Mac needs the binary. The psycopg2-binary should be replacesd with psycopg2 when possible: https://www.psycopg.org/docs/install.html#psycopg-vs-psycopg-binary.

KK-792l
@nikomakela nikomakela requested a review from charn October 6, 2021 07:58
Copy link
Contributor

@charn charn left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 :shipit:

@nikomakela nikomakela merged commit 3244c83 into master Oct 6, 2021
@nikomakela nikomakela deleted the kk-792-fix-psycopg branch October 6, 2021 08:59
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