-
Notifications
You must be signed in to change notification settings - Fork 7
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 billy project #4
Conversation
@@ -45,5 +45,6 @@ | |||
end | |||
end | |||
|
|||
command "#{install_dir}/embedded/bin/pip install --install-option=--prefix=#{install_dir}/embedded -r requirements.txt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct, on the branch we are using brache is self-contained in setup.py.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I use pip install -r
instead of pip install .
(which is python setup.py install
, I think), is because there is a line git+https://github.com/victorlin/zope.sqlalchemy.git@bugfix-keep-session-with-flush
in Billy requirements.txt
You can see it there
https://github.com/balanced/billy/blob/master/requirements.txt#L2
https://github.com/balanced/billy/blob/master/setup.py#L14
I have a patch for a bug to the package zope.sqlalchemy
. And unfortunately, python setup.py install
doesn't support git cloning dependency. So I have to run it with pip install -r requirements.txt
first.
The good news is I just realized the new version which contains my patch has been released. So I can install the package from PyPi instead of my own github repo now.
I can update billy and remove that line now. But just curious, would there be any side effect to run pip install -r requirements.txt
instead of python setup.py install
while building the package? I think all they do are the same, install necessary files into the building folder and package them later. Just like to know, in case if we have similar situation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@victorlin we run an internal pypi as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its totally fine to do a -r requirements.txt
as part of the build if thats what you want to do, will totally work fine. The internal package server is automatically setup for pip as part of the build environment though, so that part is taken care of for you :-) If that forked package is only ever used for billy and not other internal stuff then this seems fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mahmoudimus oh, so in situation like to have your own customized/patched third-party package, you can upload them to our own PyPi and just add them in the requirements.txt without a problem. So that's what the pip index-url configuration for. Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, you can add them to either setup.py or reqs.txt if you upload them to the devpi server (pypi.vandelay.io).
@coderanger Can you review this branch and merge it if you think it's okay? Thanks |
|
||
name 'billy' | ||
|
||
dependency 'setuptools' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to use this
hmmm... odd, it works on my local CI, but looks like it depends on system libpq on that builder server https://ci.vandelay.io/job/billy-build/4/console looking into why |
@victorlin I ran into this issue today. Basically, pip is configured to use wheels (which is what I uploaded to devpi) so it will try to install the uploaded psycopg2 via wheels. You need to change the installation instruction to --no-use-wheel. See: 8dc519e #{install_dir}/embedded/bin/pip install --no-use-wheel |
@victorlin is this ready 2 go ? cna you rebase and let's merge it in! |
@mahmoudimus okay, rebased |
…ot necessary to do it here again
No description provided.