-
Notifications
You must be signed in to change notification settings - Fork 49
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
Install script for panoptes dependencies #129
Conversation
Wilfred who is doing a similar exercise with the Raspberry Pi.
…ls so far because fitsio headers can't be found. Also added download of index files.
and command-line flags to disable each of the various types of dependencies from beig installed. Still need to remove excess crap at end of file, and to resolve a problem running pytest.
Ready for review AND testing (perhaps in a docker container).
Codecov Report
@@ Coverage Diff @@
## develop #129 +/- ##
========================================
Coverage 84.39% 84.39%
========================================
Files 35 35
Lines 2345 2345
Branches 281 281
========================================
Hits 1979 1979
Misses 285 285
Partials 81 81 Continue to review full report at Codecov.
|
Add libcfitsio-bin to apt packages.
Another TODO: it would be nice to not install astrometry if it hasn't changed, but they don't seem to expose a convenient way to determine the installed version. Sigh. May work around by writing a version number file from the install script. |
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.
I have not actually run this yet but am heading over to lab so will try it soon.
.travis.yml
Outdated
@@ -59,7 +59,7 @@ install: | |||
- source activate test-environment | |||
|
|||
- cd $TRAVIS_BUILD_DIR | |||
- pip install -r requirements.txt | |||
- pip install -r scripts/install/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.
Just a note to move requirements.txt
back to root
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.
Done.
# dcraw decodes Canon raw images, and gphoto2 takes pictures with | ||
# Canon DSLRs, among others. | ||
dcraw | ||
gphoto2 |
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.
gphoto2-updater is a nice install script that we might want to use if we need a newer version.
https://github.com/gonzalo/gphoto2-updater
The apt version might be simpler but I haven't had issue with the script.
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.
I can add a todo, but if we're happy enough with the package version, I think we should stick with it for simplicity.
# of these. | ||
# TODO(jamessynge): Discuss these with Wilfred as an additional way to get | ||
# temperature data. | ||
# lm-sensors |
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.
Not entirely sure we need this right now because as you say we need a reliable way to do it. Currently not being used although it was intended to be.
I guess would prefer to leave out of comments for now and maybe make a separate Issue
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.
Added an issue.
|
||
# These are used if you want to setup SSH access into the computer. | ||
# openssh-client | ||
# openssh-server |
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.
I would think we will always want this.
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.
Done
# Graphviz is used for rendering the state machine of POCS. | ||
# TODO(wtgee): Are there any other uses? | ||
graphviz | ||
libgraphviz-dev |
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.
No other uses and sometimes the install can go wrong. We detect if installed and don't plot otherwise.
Really the state machine images only need to be generated once ever by any machine and then can be stored. We might just want to include the images themselves in the repo. Would become an issue if state configuration changed.
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.
Removed TODO.
function install_mongodb() { | ||
# This is based on https://www.howtoforge.com/tutorial/install-mongodb-on-ubuntu-16.04/ | ||
# Note this function does not configure mongodb itself, i.e. no users or | ||
# security settings. |
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.
IIRC the default install only exposes on localhost. No authentication or authorization has been added yet.
[Service] | ||
User=mongodb | ||
Group=mongodb | ||
ExecStart=/usr/bin/mongod --quiet --config /etc/mongod.conf |
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.
We should also specify a --dbpath
(or alter the mongod.conf
). I've put this in $PANDIR/data
previously. Our current setup writes a lot of entries to the database and it grows very large (usually needs > 4GB to run). On PAN001 all of $PANDIR is on a separate mount from the rest of /var
(and everything else)
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.
Note also that mongo does not require you to actually create a collection (read: database) before using it. You can just start writing to something and it will store properly
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.
Not surprisingly, I didn't know that info about --dbpath.
Does mongodb allow multiple dbpath directories (i.e. can we arrange for our collections to be in /var/panoptes)?
No change made yet.
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.
There is some info (for instance, here) but I think the better answer would be to run what we want in a container.
local -r the_script="${PANDIR}/tmp/miniconda.sh" | ||
echo_bar | ||
echo | ||
echo "Installing miniconda" |
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.
Just to be good sports we should mention here that there is a default acceptance of the license agreement.
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.
Added link to license.
# See http://astrometry.net/doc/readme.html#getting-index-files | ||
# for info on the index files you might need. | ||
if [[ "${DO_ASTROMETRY_FULL_INDICES}" -eq 1 ]] ; then | ||
if [[ ! -f index-4107.fits ]] ; then |
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.
Just a note that if download/install gets interrupted this will not work properly second time around.
We also have $POCS/pocs/utils/data.py
that does this same checking and is run before tests.
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.
I notice that it installs more index files.
Do you have a suggestion regarding the interruption? Explicitly do the download of each?
Or just run $POCS/pocs/utils/data.py near the end of the install.
FWIW, I'd like to eliminate all downloading from tests.
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.
fi | ||
|
||
|
||
if [[ "${DO_PIP_REQUIREMENTS}" -eq 1 ]] ; then |
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.
Let's actually create a separate panoptes environment in conda and use that. I've just started calling mine panoptes-env
. So we will want to create it, install pip items there, and optionally set it as the default environment for the system (via ~/.profile
). Something like:
$ conda create -y -n panoptes-env python=3
# Wait
$ source activate panoptes-env
(panoptes-env) $ pip install -U pip
(panoptes-env) $ pip install -r requirements.txt
(panoptes-env) $ echo "source activate panoptes-env" >> ~/.profile # Assume we always want
Note also we can create a conda environment file that can do all this and have the right collection of files. That is what I was doing in the consolidated branch but was having hassle. We should get this install script working first and then later worry about the environment file. But I do think we want to create/use a specific panoptes environment as outlined above.
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.
Why do you say "# Wait" after conda create?
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.
I just meant that it takes a minute and there is output between those two steps.
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.
Done (well, no wait included).
We should submit a patch to astrometry.net to do this. It annoys me when a |
python-pyfits | ||
swig | ||
zlib1g-dev | ||
|
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.
Looks like byobu
comes with Ubuntu server but not desktop. Let's put it in here to make sure we have it. It will pull in tmux
also (I think we should default to byobu but will make comments in #97 )
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.
Added byobu & tmux (explicitly).
Emit message reminding the user that PATH needs to be updated in current shell.
Create a conda environment for all the python packages, etc. Install python packages before building astrometry.net, as it uses numpy.
Create panoptes-env in conda.
Closing and re-opening PR to trigger another travis build. I reviewed the failure, and it smells like a problem interacting with temp files and subprocesses, rather than with the commits, so I want to see if it is reproducible as is. |
Well, that was a false hope. The failure was essentially identical:
|
pocs/utils/data.py
Outdated
df = data.download_file(url) | ||
try: | ||
shutil.move(df, dest) | ||
except OSError as e: | ||
print("Problem saving. (Maybe permissions?): {}".format(e)) | ||
|
||
for i in range(4112, 4119): |
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.
Just reproduced the test failure on my local and it's because we are missing 4110 and 4111 index files. The fits files we use for testing have a small field-of-view, which is what those index files will search. 10 and 11 are fairly large (24MB and 10MB) so this is a bit annoying as we only really need them for the tests.
I also removed the 42xx from my local and all tests still pass. The 41xx are wide-field so should be all that we need although I suppose it doesn't hurt to have all of them.
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.
OK. Can we replace the file used for testing to a wide field also?
I've discovered that writing to .profile didn't leave me with a working profile (activate was apparently in the wrong place). Will investigate further. |
images that have narrow fields, unlike regular Panoptes images.
This installs:
Has code for building and installing latest version of cfitsio, but that is currently disabled in favor of using the latest linux package for cfitsio. Avoiding the build because I was finding some problems with using astrometry.
Partially addresses #115. Would still like to include clone of git repos, and properly configuring linux for ssh, mongodb for security, etc.