Add noarch support to the installer #9

lbdreyer · 2018-02-07T12:52:01Z

Conda introduced noarch python packages. You can tell that the binary is noarch as it will have a 'link.json' with information about the noarch in the info directory of the binary (once it's been extracted).

To install a noarch python package, there are extra steps that need to be taken:

move files from site-packages -> lib/python2.7/site-packages
move file from python-scripts -> bin/
compile pyc files
create entrypoints

This PR modifies the 'link' function which gets run when we install RPMs. now it checks that the package is noarch and if so does the above 4 extra steps. These extra steps are done using copies of the relevant functions taken from the most recent of conda. I have slightly had to modify these.

lbdreyer · 2018-02-07T12:55:02Z

Outstanding issues:

not tested with python3 or packages with entrypoints
** tested with python3 and jupyter-core and anaconda-client which both have entrypoints
no tests
no support for noarch on windows
** gonna keep it no support for noarch on window, and include a check that raises an exception
when the metadata is created (the json file for each package which gets put in the conda-meta directory of the environment), in the list of files, it doesn't include the new pyc files and the entrypoints
** these have now been added
When an entrypoint is created, it doesn't do the special handling of shebang longers than 127 characters
** we need it, I tested it
** this is needed so I've put it back
The order of the noarch specific installation steps:
** The order is 1) move site_packages, etc into environment 2) create entypoints 3) compile pyc files

lbdreyer · 2018-02-07T13:20:02Z

conda_rpms/install.py

+        match = re.search('python-(\d+.\d+).\d+-\d+', dist)
+        if match:
+            py_ver = match.group(1)
+    return py_ver


I used this method as that is how conda
I couldn't copy a function from conda so I just wrote a simplified version of what is being done (without all the extra conda-ness)

An explanation since regexp is horrible:
This searches for a package called something like python-2.7.6-3 and from that returns the group 2.7
the \d means digits (0-9), the + means there can be any number of digits and so it can also match for things like python-412.23.1-5645654654 for which it would return 412.23 as the python version

pelson

Looking good. I see what you mean about struggling to integrate it tidily, but this was always going to be quite an invasive change I think.

pelson · 2018-02-07T16:21:27Z

conda_rpms/install.py

+    for dist in linked(prefix):
+        match = re.search('python-(\d+.\d+).\d+-\d+', dist)
+        if match:
+            py_ver = match.group(1)


Personally, I'd put an else on this for loop to catch an issue with python not being installed.

Alright
I have encountered the question of what to do with errors frequently in this.
I don't want an exception to be raised as then that would break the RPM installation (I think). I would prefer it to just fail silently, but then for us to be able to see why it failed by looking at logs (not that we can see them).

I'd put an else on this for loop to catch an issue with python not being installed

How would you handle Python not being installed.
Is it enough to add something to the log that say "Python not already linked in the env"?

pelson · 2018-02-07T16:22:27Z

conda_rpms/install.py

+    noarch = False
+    # If the distribution is noarch, it will contain a `link.json` file in
+    # the info_dir
+    link_json = join(info_dir, 'link.json')


This seems shaky. Can't we inspect the metadata in info.json (or whatever it is called)? The bit that says noarch: python?

We can check that noarch: python exists in the index.json of the metadata but we do need to get the information about the entrypoints from the link.json

pelson · 2018-02-07T16:24:04Z

conda_rpms/install.py

+                                                         target_site_packages)
+                dst = join(prefix, noarch_f)
+            else:
+                dst = join(prefix, f)


Is this really all we need to do for noarch: whatever packages? Can we be a bit more defensive on this?

Not sure what you mean by this.
There is some more noarch specific installtion steps later on (entrypoints and py compilation) but they have to be done after this

lbdreyer · 2018-02-08T00:43:16Z

conda_rpms/install.py

@@ -516,18 +697,55 @@ def link(pkgs_dir, prefix, dist, linktype=LINK_HARD, index=None, target_prefix=N
        return

    source_dir = join(pkgs_dir, dist)
-    if not run_script(dist, 'pre-link', prefix, target_prefix):
+    if not run_script(prefix, dist, 'pre-link', target_prefix):


This was just a bug

(cherry picked from commit c40f1b49bb2624d416f88daea37e0cce0f15731f)

bjlittle · 2018-04-03T15:33:38Z

conda_rpms/build_rpm_structure.py

@@ -143,11 +139,29 @@ def create_rpmbuild_for_tag(repo, tag_name, target, config):
    with open(spec_fname, 'r') as fh:
        env_spec = yaml.safe_load(fh).get('env', [])
    create_rpmbuild_for_env(manifest, target, config)
-    pkgs = [pkg for _, pkg in manifest]
+
+    index = conda.fetch.fetch_index(list(set([url for url, pkg in manifest])),


The unpacked pkg is not used, so you could replace it with _ ...

bjlittle · 2018-04-03T15:42:29Z

conda_rpms/build_rpm_structure.py

+        anaconda_url = 'https://conda.anaconda.org/'
+        if url.startswith(anaconda_url):
+            url = url[len(anaconda_url):]
+        dists.append('::'.join([os.path.dirname(url), pkg]))


So, if I understand this right ... your influencing the sort here by chopping out the https://conda.anaconda.org/ from the URL to equalize with items in the manifest that have a channel name rather than a full URL? i.e. https://conda.anaconda/org/wibble/wobble/pkg becomes wibble/wobble::pkg ? and that makes it a fairer sort against conda-forge::pkg ...

This doesn't influence the sort
The function that does the ordering requires an input to be of the format channel::pkg. If a channel is not on anaconda, it uses the full url. If the channel is on anaconda, it uses just the channel name.
It is like when you do a conda list. Packages from the defaults channel or conda-forge say defaults or conda-forge however other channels (e.g. our internal channels) have the full url.

bjlittle · 2018-04-03T15:43:24Z

conda_rpms/install.py

 import shlex
-from os.path import abspath, basename, dirname, isdir, isfile, islink, join
+from os.path import abspath, basename, dirname, exists, isdir, isfile, islink, \
+    join, split, splitext


@lbdreyer Do we care about sorting the import order here?

Not sure. It is a copy of the conda/install.py and I wanted to make minimal changes, but I also wanted to have minimal code necessary (rather than it be some long thing).
But I think we could reorder the imports, as long as we put in a comment saying that we did that.

bjlittle · 2018-04-03T16:06:23Z

conda_rpms/install.py

+        * changed log.trace -> log.info (log.trace not supported)
+
+    """
+    command = [python_exe_full_path, '-Wi', '-m', 'py_compile', py_full_path]


@lbdreyer This isn't exactly the same ... kinda is, but missing the quotation around python_exe_full_path and py_full_path when substituted into the command i.e.

command = '"%s" -Wi -m py_compile "%s"' % (python_exe_full_path, py_full_path)

So within conda, if we had python_exe_full_path = "wibble" and py_full_path = "wobble" we'd get a command of "wibble" -Wi -m "wobble" BUT the modified change we get wibble -Wi -m wobble ... don't know if this really matters or not, perhaps not.

bjlittle · 2018-04-03T16:12:00Z

conda_rpms/install.py

+    subprocess.call(command)
+
+    if not isfile(pyc_full_path):
+        log.info('{} failed to compile correctly'.format(pyc_full_path))


@lbdreyer The original log message was kinda more helpful i.e.

message = """ pyc file failed to compile successfully python_exe_full_path: %()s\n py_full_path: %()s\n pyc_full_path: %()s\n """ log.info(message, python_exe_full_path, py_full_path, pyc_full_path)

Did we want to add more detail here incase it barfs?

bjlittle · 2018-04-03T16:14:44Z

conda_rpms/install.py

+
+def parse_entry_point_def(ep_definition):
+    """
+    Copy of conda/core.path.py:parse_entry_point_def at


This is conda/common/path.py:parse_entry_point_def

bjlittle · 2018-04-03T16:20:38Z

conda_rpms/install.py

+                     br'(?:[ ]*)'  # allow spaces between #! and beginning of the executable path
+                     br'(/(?:\\ |[^ \n\r\t])*)'  # the executable is the next text block without an escaped space or non-space whitespace character  # NOQA
+                     br'(.*)'  # the rest of the line can contain option flags
+                     br')$')  # end whole_shebang group


@lbdreyer make this global to this module i.e. remove the definition of SHEBANG_REGEX here. Is there a reason why you've made it local?

Only made it local because thought it didn't need to be global and I thought it would be better if it was closer to where it was being used to make it clear it was part of the noarch changes but I think there is a good place I can move it to.

bjlittle · 2018-04-03T16:22:55Z

conda_rpms/install.py

+    at be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe.
+
+    Modifications included:
+        * removed mode check for non-binary shebang


@lbdreyer Why can't we support mode?

It seemed a bit pointless. mode is included in the original function because the function would have been reused in lots of places.
In this install.py script, this function is only called by create_python_entry_point and if you look at the original function, in the call to create_python_entry_point it sets the mode to be FileMode.text so there just doesn't seem to be much point

bjlittle · 2018-04-03T16:25:13Z

conda_rpms/install.py

+    at be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe.
+
+    Modifications included:
+        * Removed check that the entry point already exists


@lbdreyer Why did you remove the check?

I can't remember. I will put it back.

I was trying to keep the code as minimal but it is actually quite a small addition

bjlittle · 2018-04-03T16:26:05Z

conda_rpms/install.py

+
+        if hasattr(shebang, 'decode'):
+            shebang = shebang.decode()
+


@lbdreyer Nuke the blank line

bjlittle · 2018-04-03T16:29:32Z

conda_rpms/install.py

+    return target_full_path
+
+
+def get_python_version(prefix):


@lbdreyer Is this your own function or is it sourced from conda?

Mine, if it was sourced from conda I stated that in the function docstring

Cool, thanks!

bjlittle · 2018-04-03T16:58:18Z

conda_rpms/install.py

+    with open(join(info_dir, 'index.json'), 'r') as fh:
+        index_data = json.loads(fh.read())
+    if 'noarch' in index_data:
+        noarch = index_data['noarch']


See https://conda.io/docs/user-guide/tasks/build-packages/define-metadata.html#architecture-independent-packages

It is possible that noarch could be set to python or generic. I don't think that we'd care about generic, but you never know.

Also, do we support the legacy noarch_python: True recipe YAML ... if so, then we should have:

if 'noarch' in index_data: noarch = index_data['noarch'] elif 'noarch_python' in index_data: noarch = index_data['noarch_python'] if noarch: if on_win: raise ValueError(...) ....

bjlittle · 2018-04-03T16:59:47Z

conda_rpms/install.py

@@ -546,6 +777,30 @@ def link(pkgs_dir, prefix, dist, linktype=LINK_HARD, index=None, target_prefix=N
                log.error('failed to link (src=%r, dst=%r, type=%r, error=%r)' %
                          (src, dst, lt, e))

+        # noarch package specific installation steps
+        if noarch == 'python':


@lbdreyer If you agree with the above comment then it might be best if this was just:

if noarch:

bjlittle · 2018-04-03T17:13:46Z

conda_rpms/install.py

+                dst = join(prefix, noarch_f)
+                all_files.append(noarch_f)
+            else:
+                dst = join(prefix, f)


@lbdreyer Add a comment to make it crystal clear that this if branch is for non-noarch.

bjlittle · 2018-04-03T17:18:08Z

@lbdreyer I'm kinda done now.

I'll spin up early tomorrow morning and go over the testing, but I think this is looking really, really good! Well done!

I've made a bunch of comments, but they're all really pretty minor.

Also, it's interesting that dask et al on conda-forge is noarch but it's not noarch on anaconda ... hmm curious, huh?

lbdreyer · 2018-04-03T21:44:44Z

I have put a commit with changes based on the reviews actions so far

bjlittle · 2018-04-04T08:21:01Z

conda_rpms/tests/integration/gitenv_create_rpmbuild.sh

@@ -7,7 +7,7 @@ export INSTALL_ROOT=/opt/conda
 REPO_ROOT=$(cd "$(dirname ${0})/../../.."; pwd;)

 # Test conda-gitenv approach.
-cat << EOF | docker run -i \
+cat << 'EOF' | docker run -i \


@lbdreyer Careful.

In this case we do want variable substitution to happen in the here document, as it makes reference to environment variables OUTDIR, ENV_REPO and INSTALL_ROOT.

Also, note that the PATH environment variable is escaped, on line 19, so that will be substituted within the container at runtime.

So, don't quote EOF

bjlittle · 2018-04-04T08:26:39Z

conda_rpms/tests/integration/gitenv_install.sh

+wget https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh --no-verbose
+bash Miniconda3-latest-Linux-x86_64.sh -b -p ${MINICONDA_DIR} && rm -f Miniconda*.sh
+
+export PATH=$MINICONDA_DIR/bin:$PATH


export PATH=${MINICONDA_DIR}/bin:${PATH}

lbdreyer · 2018-04-04T08:32:56Z

conda_rpms/install.py

+    """
+
+    if os.path.lexists(target_full_path):
+        warnings.warn('Entrypoint {} already exists.'.format(target_full_path))


I did this so that it would keep going even if the entrypoint already exists.
That way the rpm installation doesn't just fail, but if something does go wrong we can look at the logs and see that this had happened

bjlittle · 2018-04-04T08:34:14Z

conda_rpms/tests/integration/gitenv_install.sh

+# Get the name of the noarch_package, e.g. tqdm-4.19.8-py_0
+noarch_tqdm=$(basename $(find $MINICONDA_DIR/pkgs -type d -name "tqdm-*-py*"))
+echo $noarch_tqdm
+python /repo/conda_rpms/install.py --pkgs-dir=$MINICONDA_DIR/pkgs --prefix=$MINICONDA_DIR/envs/test-env --link $noarch_tqdm


@lbdreyer Fancy enclosing all environment variables in {...} ?

bjlittle · 2018-04-04T08:38:27Z

conda_rpms/tests/integration/gitenv_install.sh

+# ----------------------------------------------------
+# This will ensure that the noarch package and all it's dependencies are
+# downloaded and in the cache.
+conda create -n test-env tqdm -c conda-forge


@lbdreyer Did you want to be explicit about the version of Python used here? As you're relying on it being Python3.6 in line 55.

So we have:

conda create -n test-env -c conda-forge tqdm=VERSION python=3.6

Agreed to pin the tqdm version to what we need and want

bjlittle · 2018-04-04T08:44:30Z

conda_rpms/tests/integration/gitenv_install.sh

+conda create -n test-env tqdm -c conda-forge
+source activate test-env
+conda uninstall tqdm
+source deactivate


@lbdreyer I'm assuming that you're installing and then uninstalling just to populate the package cache with the tqdm tarball?

You could do a quick check here to ensure that tqdm is not in the site-packages directory and its associated entry point has been uninstalled - this ensures that the following install.py test is valid.

bjlittle · 2018-04-04T09:17:05Z

conda_rpms/tests/integration/gitenv_install.sh

+python -c "import tqdm"
+
+echo 'Check the entrypoint exists'
+which tqdm


@lbdreyer If the which or python -c "import tqdm" fail, does this make the whole test fail?

Just want to make sure that as far as circle-ci is concerned, the test doesn't always pass, just because it ran.

So i tested this.
You can see that a bad which fails the whole job in the output here:
https://circleci.com/gh/lbdreyer/conda-rpms/66

and you can see that a bad python -c "import ..." fails here:
https://circleci.com/gh/lbdreyer/conda-rpms/67

bjlittle · 2018-04-04T09:36:16Z

@lbdreyer Just checked what happens for uninstalling, and basically for a tag it nukes the complete directory from the install root i.e. /opt/scitools/environments/default/YYYY_MM_DD.

It's not that we're supporting uninstalling individual packages, but uninstalling complete environments, which is easier.

So, that said, I think we're all good for uninstalling - nothing extra to do.

bjlittle · 2018-04-04T09:50:50Z

conda_rpms/tests/unit/build_rpm_structure/test_create_rpmbuild_for_tag.py

+                # Create fake env.spec and manifest files
+                with open(temp_repo + '/env.spec', 'w') as env_spec_file:
+                    env_spec_file.write(ENV_SPEC)
+                with open(temp_repo + '/env.manifest',


@lbdreyer Use os.path.join here and above in line 34

bjlittle · 2018-04-04T10:07:36Z

conda_rpms/tests/unit/build_rpm_structure/test_create_rpmbuild_for_tag.py

+                    # Parse the lines of the spec file that are formatted as
+                    # `  {INSTALL} xz-5.2.3-0\n`
+                    if line.startswith('  ${INSTALL}'):
+                        result_order.append(line.split(' ')[3][:-1])


@lbdreyer We're kinda being held to ransom by the format of the line and readlines tacking on a \n, which make the indexing kinda non-intuitive. How about:

for line in fh.readlines(): line = line.strip() if line.startswith('${INSTALL}'): result_order.append(line.split(' ')[1])

bjlittle · 2018-04-04T10:09:06Z

@lbdreyer Okay, over to you ...

lbdreyer · 2018-04-04T17:03:11Z

Okay @bjlittle I have now dealt with all the review actions. I am going to run the testing in the docker container

bjlittle · 2018-04-04T17:56:27Z

circle.yml

-    - ./conda_rpms/tests/integration/gitenv_create_rpmbuild.sh
-    - ./conda_rpms/tests/integration/gitenv_build_rpms.sh
+    #    - ./conda_rpms/tests/integration/gitenv_create_rpmbuild.sh
+    #- ./conda_rpms/tests/integration/gitenv_build_rpms.sh


Yup, makes sense these integration tests are failing anyways 👍

Argh sorry. I only actually did this cause I didn't want to have to wait for these tests to run. I'll un-comment these so it's back to how it was

bjlittle · 2018-04-04T18:02:16Z

conda_rpms/install.py

+        if index_data['noarch_python'] is True:
+            noarch = 'python'
+
+    if noarch:


@lbdreyer I guess this should be if noarch == 'python' then...

bjlittle · 2018-04-04T18:11:07Z

conda_rpms/tests/unit/build_rpm_structure/test_create_rpmbuild_for_tag.py

+                with open(os.path.join(temp_repo + '/env.spec'), 'w') as \
+                        env_spec_file:
+                    env_spec_file.write(ENV_SPEC)
+                with open(os.path.join(temp_repo + '/env.manifest'), 'w') as \


@lbdreyer Note that you don't need the leading / as os.path.join will do this for you.

bjlittle · 2018-04-04T18:19:01Z

@lbdreyer After you're satisfied all is well after testing (apologies, added another couple of minor comments) I'll squash and merge! 😄

How exciting!

lbdreyer · 2018-04-04T22:32:03Z

@bjlittle Okay. I've tested it now and left the container running if you'd like to have a look.
The module files won't work as I haven't got module installed in the container but you can find the environments in the same location as they are on the desktops

bjlittle · 2018-04-05T07:57:02Z

conda_rpms/tests/unit/build_rpm_structure/test_create_rpmbuild_for_tag.py

+                with open(os.path.join(temp_repo + 'env.spec'), 'w') as \
+                        env_spec_file:
+                    env_spec_file.write(ENV_SPEC)
+                with open(os.path.join(temp_repo + 'env.manifest'), 'w') as \


@lbdreyer I almost merged, but thought I give the code just one last skim through ...

This should be os.path.join(temp_repo, 'env.spec') and os.path.join(temp_repo, 'env.manifest') ... we need to use , not +.

But surely, this would have caused the testing to fail?

@lbdreyer Perhaps I'm being dense (again) but I can't see how the unit tests are being run as any part of the CI, can you?

I assume that you can run the unit tests within your development environment?

If you need to run the unit tests by hand, then just confirm that they work then create an issue to ensure that we add unit testing to the CI (via travis-ci I guess) at a later date.

I'm keen for us to get this merged 👍

Good call, I hadn't done the os.path.join correctly
I fixed it in the last commit and you can see the unit test passed:

$ python -m unittest -v conda_rpms.tests.unit.build_rpm_structure.test_create_rpmbuild_for_tag test_sorted_order ... ok ---------------------------------------------------------------------- Ran 1 test in 5.960s OK

I ran all the tests and I got one failure:

.....E........... ====================================================================== ERROR: test_pkg_all_linked (conda_rpms.tests.unit.build_rpm_structure.test_create_rpmbuild_for_env.Test) ---------------------------------------------------------------------- Traceback (most recent call last): File "../conda-rpms/conda_rpms/tests/unit/build_rpm_structure/test_create_rpmbuild_for_env.py", line 19, in test_pkg_all_linked with patch(func, return_value=zip(*self.pkgs)[1]): TypeError: 'zip' object is not subscriptable ---------------------------------------------------------------------- Ran 17 tests in 13.076s FAILED (errors=1)

I have raised the issue for unit tests being added to CI:
#11

bjlittle · 2018-04-05T09:46:49Z

@lbdreyer Well done! 🍺 🎉 🎈 👍

pelson · 2018-04-11T09:38:31Z

Great job on this @lbdreyer. Pretty intense stuff, and it looks really comprehensive 👍 🎉

lbdreyer commented Feb 7, 2018

View reviewed changes

pelson reviewed Feb 7, 2018

View reviewed changes

lbdreyer force-pushed the noarch_installer branch 2 times, most recently from fecd087 to faa4e91 Compare February 8, 2018 00:41

lbdreyer commented Feb 8, 2018

View reviewed changes

lbdreyer force-pushed the noarch_installer branch 3 times, most recently from 1ab463f to feb1e81 Compare February 8, 2018 13:40

lbdreyer force-pushed the noarch_installer branch from ab2f125 to b595ff3 Compare March 27, 2018 11:08

lbdreyer added 2 commits March 27, 2018 12:40

Add noarch support to the installer

3fba61a

Sort dependencies of packages

49216f2

(cherry picked from commit c40f1b49bb2624d416f88daea37e0cce0f15731f)

lbdreyer force-pushed the noarch_installer branch from b595ff3 to 49216f2 Compare March 27, 2018 11:44

lbdreyer mentioned this pull request Mar 27, 2018

Sort packages order in the RPM based on their dependencies #8

Closed

lbdreyer force-pushed the noarch_installer branch from a188d39 to c4e993d Compare March 29, 2018 11:30

Add message explainging whats happening in test

bdff5cf

lbdreyer force-pushed the noarch_installer branch from 1ec0504 to bdff5cf Compare April 3, 2018 10:53

bjlittle reviewed Apr 3, 2018

View reviewed changes

bjlittle reviewed Apr 4, 2018

View reviewed changes

lbdreyer commented Apr 4, 2018

View reviewed changes

bjlittle reviewed Apr 4, 2018

View reviewed changes

lbdreyer force-pushed the noarch_installer branch from 9f9cec7 to a975653 Compare April 4, 2018 16:51

Review actions

3a90a41

lbdreyer force-pushed the noarch_installer branch from 6377daf to 3a90a41 Compare April 4, 2018 17:02

bjlittle reviewed Apr 4, 2018

View reviewed changes

bjlittle mentioned this pull request Apr 4, 2018

Fix CI integration tests #10

Open

Final review actions

5f52eb1

bjlittle reviewed Apr 5, 2018

View reviewed changes

fix unit tag path join

957f8a9

bjlittle merged commit fe3dd0b into SciTools-incubator:master Apr 5, 2018

Add noarch support to the installer #9

Add noarch support to the installer #9

Conversation

lbdreyer commented Feb 7, 2018 • edited Loading

lbdreyer commented Feb 7, 2018 • edited Loading

lbdreyer Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

pelson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbdreyer Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

lbdreyer Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbdreyer Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbdreyer Apr 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle Apr 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle commented Apr 3, 2018 • edited Loading

lbdreyer commented Apr 3, 2018

bjlittle Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle commented Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle commented Apr 4, 2018

lbdreyer commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

bjlittle commented Apr 4, 2018 • edited Loading

lbdreyer commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle Apr 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjlittle commented Apr 5, 2018

pelson commented Apr 11, 2018

lbdreyer commented Feb 7, 2018 •

edited

Loading

lbdreyer commented Feb 7, 2018 •

edited

Loading

lbdreyer Feb 7, 2018 •

edited

Loading

lbdreyer Feb 7, 2018 •

edited

Loading

lbdreyer Feb 7, 2018 •

edited

Loading

lbdreyer Feb 7, 2018 •

edited

Loading

lbdreyer Apr 3, 2018 •

edited

Loading

bjlittle Apr 3, 2018 •

edited

Loading

bjlittle commented Apr 3, 2018 •

edited

Loading

bjlittle Apr 4, 2018 •

edited

Loading

bjlittle Apr 4, 2018 •

edited

Loading

bjlittle Apr 4, 2018 •

edited

Loading

bjlittle commented Apr 4, 2018 •

edited

Loading

bjlittle Apr 4, 2018 •

edited

Loading

bjlittle commented Apr 4, 2018 •

edited

Loading

bjlittle Apr 5, 2018 •

edited

Loading