Squarespace / pgbedrock

Manage a Postgres cluster's roles, role memberships, schema ownership, and privileges
https://pgbedrock.readthedocs.io/en/latest/
Other
311 stars 35 forks source link

Move pinned dependencies to requirements.txt #45

Closed sourcenouveau closed 5 years ago

sourcenouveau commented 5 years ago

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.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 174


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

💛 - Coveralls
sourcenouveau commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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