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.

Sign up for GitHub

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

Jump to bottom

Fix quickstart guide for bigchaindb_driver #316

Merged
merged 2 commits into from
Jul 18, 2017
Merged

Fix quickstart guide for bigchaindb_driver #316

merged 2 commits into from
Jul 18, 2017

Conversation

muawiakh
Copy link
Contributor

  • Fixes quickstart for a fresh Ubuntu/RHEL setup,
    and installs all the relevant dependencies.

@muawiakh muawiakh mentioned this pull request Jul 17, 2017
Closed
@muawiakh
Copy link
Contributor Author

Fixes #315

@codecov-io
Copy link

codecov-io commented Jul 17, 2017 •
edited
Loading

Codecov Report

Merging #316 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #316   +/-   ##
=======================================
  Coverage   99.15%   99.15%           
=======================================
  Files           9        9           
  Lines         236      236           
=======================================
  Hits          234      234           
  Misses          2        2

ttmc
ttmc reviewed Jul 17, 2017
View reviewed changes
Copy link
Contributor

@ttmc ttmc left a comment

Choose a reason for hiding this comment

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

I don't understand why libffi-dev and libssl-dev are enough for the Python driver, because it depends on the bigchaindb package (BigchainDB Server), which depends on more OS-level packages than those two.

ttmc
ttmc suggested changes Jul 17, 2017
View reviewed changes
Copy link
Contributor

@ttmc ttmc left a comment

Choose a reason for hiding this comment

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

I made a few minor copy-editing comments.

docs/quickstart.rst Outdated

.. code-block:: bash

sudo dnf install openssl-devel
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.

Done

docs/quickstart.rst Outdated
.. code-block:: bash

sudo dnf install openssl-devel

Copy link
Contributor

Choose a reason for hiding this comment

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

Below, Python 3.5+ now becomes "Dependency 3". The following ones have to be renumbered as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

docs/quickstart.rst Outdated

OR

$ sudo pip3 install --upgrade setuptools
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this one use sudo but the one above doesn't? Same comment for the next subsection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it didn't work for me without the sudo and I was not using the virtualenv or conda, and running this on base VM. I can change that to pip3 install --upgrade setuptools as well to keep it consistent, and if the user encounters a permissions issue they can run it with sudo which is pretty straight forward. Same for the next subsection, because with virtualenv, I don't need sudo permissions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay maybe just remove all the sudo

@ttmc
Copy link
Contributor

ttmc commented Jul 17, 2017 •
edited
Loading

Don't worry about the automatic codecov test results in documentation-only pull requests. Clearly it is wrong.

@muawiakh
Copy link
Contributor Author

@ttmc because the driver + apt pull the dependencies recursively.

@muawiakh muawiakh force-pushed the fix-bdb-driver-quickstart branch from 005f273 to f332da1 Compare July 17, 2017 19:54
@ttmc
Copy link
Contributor

ttmc commented Jul 18, 2017

@ttmc because the driver + apt pull the dependencies recursively.

Okay, so when apt installs libffi-dev and libssl-dev, it also installs all the other packages. That's nice: only two packages to install.

But why, when we install the bigchaindb Python package alone, do we have to install a bunch of other OS-level dependencies? The Python Driver bigchaindb_driver package depends on the bigchaindb package, so it also depends on all of BigchainDB Server's OS-level dependencies, no?

In short, A depends on B, but A has fewer dependencies than B?

Fix quickstart guide for bigchaindb_driver
3994a44 
- Fixes quickstart for a fresh Ubuntu/RHEL setup,
and installs all the relevant dependencies.
@muawiakh muawiakh force-pushed the fix-bdb-driver-quickstart branch from f332da1 to 3994a44 Compare July 18, 2017 08:07
@muawiakh
Copy link
Contributor Author

We only need libssl-dev and libffi-dev for the server quickstart as well. Nothing more. I will update it.

@kremalicious
Copy link

kremalicious commented Jul 18, 2017 •
edited
Loading

So is there a way to have pip install bigchaindb_driver automatically install libssl-dev and libffi-dev? Would be just a bit nicer UX

@muawiakh muawiakh merged commit 5778685 into master Jul 18, 2017
@muawiakh
Copy link
Contributor Author

@kremalicious No, pip cannot. It can only install python modules as per the requirements.txt or setup.py

@codegeschrei codegeschrei deleted the fix-bdb-driver-quickstart branch March 21, 2018 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttmc ttmc ttmc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@muawiakh @codecov-io @ttmc @kremalicious @sbellem