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

Move pinned dependencies to requirements.txt #45

Merged
merged 2 commits into from
Aug 30, 2018
Merged

Move pinned dependencies to requirements.txt #45

merged 2 commits into from
Aug 30, 2018

Conversation

sourcenouveau
Copy link
Contributor

Pinned dependencies in setup.py make it difficult to use pgbedrock in environments that are shared with other packages. If a user upgrades a dependency and tries to execute pgbedrock they can get a pkg_resources.DistributionNotFound error.

This pull request relaxes the dependency constraints in setup.py and adds requirements.txt. The relaxed constraints in setup.py enable the user to execute pgbedrock in more environments, while the pinning in requirements.txt provides users and developers with a "known good" environment.

pgbedrock seems to work correctly with the latest versions of all dependencies. I do not know whether there are any known minimum version requirements for any of the dependencies, but if there are please add them to setup.py.

Relaxed the dependency constraints in setup.py, and moved pinned versions to requirements.txt.
@coveralls
Copy link

coveralls commented Aug 16, 2018

Pull Request Test Coverage Report for Build 174

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.6%) to 95.616%

Totals Coverage Status
Change from base Build 167: -1.6%
Covered Lines: 567
Relevant Lines: 593

💛 - Coveralls

@sourcenouveau
Copy link
Contributor Author

@sturzhav, @cpdean:

I saw @zcmarine's comment on #40 that he's no longer with Squarespace. Any chance that we'll be able to get this PR merged soon? I'm not able to use pgbedrock in my environment due to this dependency issue.

@cpdean
Copy link
Contributor

cpdean commented Aug 27, 2018

@emddudley
Hey! This is a good idea and I want to merge it but I need to change the Dockerfile to use the requirements.txt so that the image we publish will be reproducible. Would you like to add that or could I lob a commit onto your PR?

Adding a copy of the requirements txt and pip install -r requirements.txt before installing the package in the container should do the trick. I've tested this locally by making my Dockerfile look like this in your branch:

FROM python:3.6

VOLUME /opt
WORKDIR /opt

COPY setup.py /opt/
COPY requirements.txt /opt/
RUN pip install -r requirements.txt
COPY pgbedrock /opt/pgbedrock
RUN pip install .

ENTRYPOINT ["pgbedrock"]

Thanks again for submitting this PR! It will make pgbedrock easier for people to use directly and install.

@cpdean
Copy link
Contributor

cpdean commented Aug 30, 2018

other than coveralls being awful and counting non-python file lines in its code coverage calculation, this looks good!

Thanks! I will try to get a release of this out for you in a bit.

@cpdean cpdean merged commit 62e536b into Squarespace:master Aug 30, 2018
@cpdean
Copy link
Contributor

cpdean commented Aug 30, 2018

Hey @emddudley ,

The version with your changes is now live on pypi.

- ❯❯❯ pip install pgbedrock
...
- ❯❯❯ pip freeze
Cerberus==1.2
click==6.7
Jinja2==2.10
MarkupSafe==1.0
pgbedrock==0.3.2
psycopg2==2.7.5
PyYAML==3.13

@sourcenouveau
Copy link
Contributor Author

Excellent, appreciate it! I'll try it out next week.

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