swinbank, would you please review this?

There are changes to all four packages (datamodel, obs_pfs, drp_stella_data and drp_stella), but these changes are all identical and consist solely of the machinery necessary to checkout and use the new pipe2d package under Travis. The reason for this approach is so that we can do more of an integration test (testing that all packages work together and the tutorial isn't broken) rather than just unit testing (testing that a single package works). The down side is that if there are changes in multiple packages, what is essentially the same test will fire multiple times (one for each package); I don't know how to get around that, and I hope that it won't be a problem because we have a small number of packages. To further reduce the load on Travis, I have push testing disabled, and only trigger testing on pull requests. Travis limits the log file to 4 MB so I have to trap the regular output in the course of the build and only display the last 100 lines of that output when done. This means there isn't a clear indicator of build progress, but in my experience each test takes around 30 minutes; most of this appears to be build time (I'm hoping we can reduce it by simplifying drp_stella).

The pipe2d package is in the process of being moved (INFRA-87) from my personal GitHub (https://github.com/PaulPrice/pipe2d) to the Subaru-PFS organisation on GitHub, and possibly renamed at the same time. It contains scripts to build the pipeline and run the tutorial, plus the stuff needed to integrate with Travis-CI. It is the authoritative source for the machinery in the other packages which grab pipe2d to run the build. The install_pfs.sh script can be used to build the pipeline on your own system, e.g., /path/to/pipe2d/install_pfs.sh /path/to/install/pfs . Once that's completed, you can load the environment by sourcing /path/to/install/pfs/pfs_setups.sh .

Comments on PaulPrice/pipe2d PR#1. I don't think further comments on the other repositories are necessary.

Most of my comments on the implementation are minor. However, either I've misunderstood something (quite possible!) or you aren't setting up the relevant branch to be tested properly: I reckon you need to add a $BRANCH false to line 184 of bin/install_pfs.sh.

Beyond that, my main concern is regarding how to actually use this in practice. I think I'm right in saying that it'll trigger on every PR. Does it follow that if I make a ticket branch in drp_stella, PR it, then push a corresponding branch to obs_pfs, the drp_stella test will run without the obs_pfs branch? If so, I don't think that's a disaster, but I do think it needs documentation and procedure. Building on that, it'd be helpful to add a note to http://pfs-2d-pipeline.readthedocs.io describing:

• How to use this system (e.g. only make PRs after the ticket branch has been pushed to all repositories, etc);
• How to update this system (what happens when LSST release 13 is out? Or when we need to update to a different version of a pinned package? You've built all the machinery for doing that, but a concise guide on which buttons to press would be useful.)
• Policies for use (e.g. do we now expect that PRs can only be merged following a successful CI run? If so, let's note it on http://pfs-2d-pipeline.readthedocs.io/en/latest/dev/best_practice.html).

You may be reluctant to define policy yourself (although I don't think you need be; I doubt anybody would really disagree): if you prefer open a ticket assigned to rhl or me to update the rules once the machinery is in place.

Some responses:

• I wanted to point out that testing python 3.5 and OSX is possible, but I didn't want to actually put the effort into it until it's determined that it will be useful and it's a good use of time to work on it. I've yanked the commented-out lines.
• The required argument to the git_clone function is useful and necessary because we want to be able to request a branch and yet not fail if it doesn't exist for this particular product. This may occur if I'm testing a ticket branch of one product, but there's no corresponding ticket branch for another product. Oh, I think you figured this out.
• Whoops on $BRANCH — I've used it where it should be used.
• You correctly observed that both install_pfs.sh and the Travis driver are deleting the PFS-specific packages. That's because they have different goals and may be used in different circumstances. The installation script wants to clobber existing PFS packages before the build, and it may be used outside Travis. The Travis driver wants to delete existing PFS packages after the build so we don't unnecessarily cache something we're just going to delete anyway.
• I think we could deploy the log file as part of the Travis procedure. We might also want to deploy the generated docs and maybe a Conda build (I think cloomis is setting up a Conda server). But that's going to take some extra work (e.g., mucking around with authentication), so I'd like to put it off as an upgrade option and get what's useful into play now if you don't mind? Filed PIPE2D-131.

I've made a bunch of updates based on your comments that should appear on the PR soon.

I'll update the docs.

I've updated the docs in drp_stella. Would you have a look please, swinbank?

price@price-laptop:~/PFS/drp_stella (tickets/PIPE2D-83=) $ git sub
commit b047328e86127f1349dd2e69bd5d308ecfd56e03
Author: Paul Price <price@astro.princeton.edu>
Date:   Wed Nov 23 22:34:59 2016 -0500

    gitignore: remove personal files
    
    A repo .gitignore should contain only files that might appear
    in the repo in the ordinary course of building or using the
    package.
    
    Files that are created because of individual user's workflows
    should be ignored through ~/.gitignore .

 .gitignore | 54 ------------------------------------------------------
 1 file changed, 54 deletions(-)

commit d552734948bb445ce6d644c13507b8f8fa3f21f0
Author: Paul Price <price@astro.princeton.edu>
Date:   Wed Nov 23 22:44:07 2016 -0500

    Enable Travis-CI

 .travis.yml | 28 ++++++++++++++++++++++++++++
 travis.sh   | 25 +++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

commit ad3d23efeff5d2161265dac61c2ff1ab3478c5e1
Author: Paul Price <price@astro.princeton.edu>
Date:   Tue Nov 29 16:18:35 2016 -0500

    update docs to include pipe2d
    
    We now have a script-based install and Travis.
    
    [ci skip]

 sphinx/dev/best_practice.rst     | 83 +++++++++++++++++++++++++++++++++++++++-
 sphinx/index.rst                 | 16 ++++++--
 sphinx/maintainer/maintainer.rst | 47 +++++++++++++++++++++++
 sphinx/user/getting_started.rst  | 65 +++++++++++++++++++++++++++++--
 4 files changed, 203 insertions(+), 8 deletions(-)

commit 53ebfe09dd662a4a233a5e298468463016e2ae91
Author: Paul Price <price@astro.princeton.edu>
Date:   Tue Nov 29 18:56:46 2016 -0500

    gitignore: add Sphinx-generated dir

 .gitignore | 1 +
 1 file changed, 1 insertion(+)

Note to self: I need to upgrade the .travis.yml files in the various subordinate packages.... Done!

Updated docs look great! I added a few picky comments on the PR. A couple of broken links which obviously need fixing, but also some fussing about stylistic issues in the markup which you may ignore if you wish.

I still think it's a little sad that it'll be easy to trigger Travis failures by PRing before you've pushed ticket branches to all the relevant repos, but I can't see a way around this, and I appreciate that you have documented it.

Other than that, this is very nice. Thank you, and go ahead and merge when you're ready.

Thanks swinbank!

I updated the docs following your comments.

I agree about the requirement to push before PRing being a bit annoying. An alternative would be to require the developer to explicitly trigger a Travis run when ready (e.g., by creating a PR on the pfs_pipe2d repo with an empty commit or something; or have some script use the Travis API), but I think what I've done is preferable because those failures are visible and easily remedied, while explicit triggers are easily forgotten or missed.

I'm in the process of updating things to use the new pfs_pipe2d repo (instead of my personal pipe2d repo), then I hope to merge.

I think this is good to go except for one thing: we're currently pointing the test to my personal GitHub version of obs_pfs because of concerns about disk space (INFRA-53). I think as soon as that's resolved, I can update pfs_pipe2d and merge. I'm concerned that merging as-is would generate confusion.

Turns out it works without the reduced-size obs_pfs.

Merged to master. I'm working on an e-mail to send around the group.

Dear PFS-ers,

I have just merged PIPE2D-83, which provides continuous integration (CI) through Travis. This introduces a new repo, a couple of new scripts, a new procedure and a new section in the docs.

The new repo is called pfs_pipe2d (https://github.com/Subaru-PFS/pfs_pipe2d). It currently contains scripts to install and exercise the PFS pipeline; I believe RHL has ideas on other things that might go in there.

The new scripts are in pfs_pipe2d:

• install_pfs.sh : you can use this to install the PFS pipeline, including conda, the LSST stack, and PFS packages. It should be tolerant of an existing install so you don't have to blow away your old stack to build a new one. It has some support for OSX, but I haven't tested that yet (PIPE2D-132). As part of the build, it writes a file that you can source to setup your environment. Documentation is at http://pfs-2d-pipeline.readthedocs.io/en/latest/user/getting_started.html#script-based-install
• pfs_integration_test.sh : you can use this to test the PFS pipeline. Documentation is at http://pfs-2d-pipeline.readthedocs.io/en/latest/dev/best_practice.html#command-line-use

Now that CI is available, there are some things to keep in mind:

• Travis-CI will fire when you create a pull request on GitHub (or push to an existing pull request). The CI run takes about half an hour. Please don't merge until you get a green light.
• If you are working on multiple packages in a single ticket, push the ticket branches to GitHub for all packages before opening the pull requests. This is because the CI is not just running the tests in the packages, but it's checking that the packages all work together, which they may not if all the work isn't available for the test.
• If you don't want Travis-CI to fire, you can put "[ci skip]" in your commit message.
• More details at http://pfs-2d-pipeline.readthedocs.io/en/latest/dev/best_practice.html#travis-use

I added a section with "Maintainer notes" to the docs: http://pfs-2d-pipeline.readthedocs.io/en/latest/maintainer/maintainer.html . This includes details on how to update the build script when LSST puts out a new release.

Let me know if you have any questions.

Thanks,

Paul.