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

Add pip requirements.txt and Pipfile #8

Merged
merged 4 commits into from
Jun 16, 2017
Merged

Conversation

remingtonc
Copy link
Contributor

Add a pip requirements.txt and Pipfile/.lock to be used for installation of the library dependencies.

This resolves the need to manually install requirements detailed in README.md and can be updated with specific versioning as required. Or updated to automatically use the latest version, just request the change in the current code.

Add a requirements.txt for installation via pip, as well
as Pipfile/.lock usable by Pipenv and eventual pip
Pipfile support.
Copy link
Contributor

@TimEvens TimEvens left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this. I have a few comments.

Pipfile.lock Outdated
@@ -0,0 +1,29 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I was checking https://github.com/pypa/pipfile and it appears that you do not need to supply this file as it should be generated. If it's generated, then this shouldn't be included in the repo. Is there another reason why the lock file is included?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Pipfile.lock enables deterministic builds - just meaning that the exact same code will be utilized. We can remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@remingtonc remingtonc Apr 5, 2017

Choose a reason for hiding this comment

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

pypa/pipfile#7

Per this - we should probably leave it here unless you don't care about library versioning.

Pipfile Outdated
verify_ssl = true

[packages]
python-snappy = "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add minimum version requirements?

  • python-snappy >= 0.5
  • kafka-python >= 1.3.0
  • PyYAML >= 3.11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this apply to requirements.txt as well with minimum versions instead of exact (==)?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah...

Pipfile Outdated
@@ -0,0 +1,8 @@
[[source]]
url = "https://pypi.python.org/simple"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm like 80% sure that the source should be the source for the project requirements - not the source of the project itself. I can't find any examples of Pipfiles that don't use pypi as a source - it might be too early in its development. https://github.com/pypa/pipfile#pipfile-1

I looked through the code for pipfile and Pipenv, and I can't tell for certain. Need to find a complete spec, which I'm not certain exists right now.

Pipenv automatically converts requirements.txt to
Pipfile format.
@remingtonc
Copy link
Contributor Author

@TimEvens I have removed the Pipenv related materials and added the minimum versioning requirements to requirements.txt. Installation of dependencies is now just pip install -r requirements.txt.

@TimEvens TimEvens merged commit beac555 into SNAS:master Jun 16, 2017
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.

2 participants