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.

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

Remove CYLC_DIR #3162

Merged
merged 28 commits into from
May 20, 2019
Merged

Remove CYLC_DIR #3162

merged 28 commits into from
May 20, 2019

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented May 11, 2019

Close #3133
Close #3080
Close #3150

Cylc-7 (and earlier) was installed by unpacking a release tarball into an arbitrary location, from which bin/cylc automatically set $CYLC_DIR to allow other scripts to locate modules and other files from the same release (by adding lib/cylc/ to the Python path, and for reading config files, etc.). A release tarball was the whole code base at the release revision, not just the application(s) and Python library. So, for instance, the (non-Python) test battery and many example suites etc. were automatically available to users.

Now for cylc-8 we must pip install cylc-flow into a Python site-packages library path. Consequently $CYLC_DIR is no longer a useful concept, and we can only install the application scripts, Python modules, and "package data" into site-packages. Further, package data should be kept to a minimum because it has to be accessed via the Python pkg_resources API (in case the package is installed as compressed archive) - which does not provide easy access to large numbers of files in a directory trees. (Plus site-packages is not really an appropriate place to put loads of files that are not Python modules). So...

This PR:

  • splits etc/ content into:
    • etc/ - for developers only (not installed via pip)
    • cylc/flow/etc/ - package data to be installed into the library for users (example config files, syntax files, etc.)
  • provides a new command cylc get-pkg-resources for users to list and extract package resources from the installed library
    • this is also used to extract the job.sh shell function library before sourcing it in task job scripts (also moved job.sh to the package resources dir, because it is not a Python library module)
  • moves the cylc test-battery command to etc/bin/run-functional-tests.sh - because the test battery is not appropriate package data, and is for use by developers anyway
  • removes the example suites from etc/examples (not appropriate as package data), except for the few included in the user guide, which moved to doc/src/suites. Most of these were very old anyway. Do we need undocumented example suites in a addition to the User Guide?
  • removes the tutorial section of the user guide - it requires a bunch of the now-removed examples suites, and we plan to replace it anyway with the cylc tutorial from Rose docs
  • for the test battery, etc/bin/run-functional-tests.sh now determines $CYLC_REPO_DIR based on its own location (it is up to the developer to ensure that the right Python environment is activated - i.e. the right installed library is use - that can't be determined automatically since it might not be a co-located virtual environment. (Note the test battery runs out of the repository, not out of the installed library ... but it uses the installed library to run cylc, of course)

UPDATE:

  • new global config file locations (used to be in CYLC_DIR):
/etc/cylc/flow/8.0.0/flow.rc
$HOME/.cylc/flow/8.0.0/flow.rc

(Version number required)

NOTE we still have to work out how to specify cylc location and possible virtual env requirement on remote job hosts (not a problem new to this PR branch however - also on current master). [done]

@hjoliver hjoliver added this to the cylc-8.0a1 milestone May 11, 2019
@hjoliver hjoliver self-assigned this May 11, 2019
@hjoliver hjoliver changed the title Remove cylc dir 2 Remove CYLC_DIR May 11, 2019
@hjoliver hjoliver marked this pull request as ready for review May 11, 2019 12:23
@hjoliver hjoliver requested review from kinow and removed request for kinow May 11, 2019 12:23
@cylc cylc deleted a comment May 12, 2019
@cylc cylc deleted a comment May 12, 2019
@cylc cylc deleted a comment May 12, 2019
@cylc cylc deleted a comment May 12, 2019
@cylc cylc deleted a comment May 12, 2019
@cylc cylc deleted a comment May 12, 2019
@hjoliver hjoliver requested review from kinow and oliver-sanders and removed request for oliver-sanders May 12, 2019 21:38
@cylc cylc deleted a comment May 12, 2019
@hjoliver hjoliver mentioned this pull request May 14, 2019
4 tasks
@hjoliver
Copy link
Member Author

Note unit and functional tests passed on Travis CI, but the final documentation build didn't run for some reason. Just pushing up a doc change, so will see if it happens again...

@kinow
Copy link
Member

kinow commented May 14, 2019

If one of the chunks failed and was kicked, the doc stage is normally cancelled and you have to trigger it manually.

@cylc cylc deleted a comment May 14, 2019
@hjoliver
Copy link
Member Author

hjoliver commented May 14, 2019

@kinow - all tests passed again on T-CI (after one kick for #2894 (comment)) ... but it shows as failed and says "build cancelled". Any idea what I've done wrong? (Is it to do with the doc stage being "usually cancelled" as you said above?

@hjoliver
Copy link
Member Author

(Note I've just pushed a new commit - the new pkg-resources command needed tweaking - but I expect the same to happen again).

@cylc cylc deleted a comment May 14, 2019
@kinow
Copy link
Member

kinow commented May 14, 2019

@kinow - all tests passed again on T-CI (after one kick for #2894 (comment)) ... but it shows as failed and says "build cancelled". Any idea what I've done wrong? (Is it to do with the doc stage being "usually cancelled" as you said above?

@hjoliver went to have a look at the build, and the docs stage was marked as cancelled, but one of the chunks had failed due to ./tests/cylc-poll/16-execution-time-limit.t (Wstat: 0 Tests: 4 Failed: 3)... so I kicked that build.

Then it failed for the second time with ./tests/cylc-poll/16-execution-time-limit.t (Wstat: 0 Tests: 4 Failed: 3). Third time's a charm, and now it passed. But the build was still cancelled, for fixing builds by kicking them (i.e. re-triggering) does not trick further items in the pipeline in other stages, even if they are normally initiated, in Travis.

Screen Shot 2019-05-14 at 20 42 23-fullpage

So I manually started the docs stage/job and everything passed 🎉 . I wonder if 16-execution-time-limit.t might have gotten "flakier" with our fixes for CYLC_DIR? (saw some discussion about this exact test from you & Oliver in the issue about flaky tests)

Cheers
Bruno

@hjoliver
Copy link
Member Author

But the build was still cancelled, for fixing builds by kicking them (i.e. re-triggering) does not trick further items in the pipeline in other stages, even if they are normally initiated, in Travis.

I'm still slightly confused - if the docs build is "normally cancelled", why does it need to get executed after kicking one of the main chunks? (That's new behaviour isn't it? Presumably since adding sphinx doc build to the T-CI config pretty recently?)

@hjoliver
Copy link
Member Author

So I manually started the docs stage/job and everything passed tada . I wonder if 16-execution-time-limit.t might have gotten "flakier" with our fixes for CYLC_DIR? (saw some discussion about this exact test from you & Oliver in the issue about flaky tests)

It does seem to have gotten flakier, I've had to kick that chunk a lot lately. Hard to see what about these changes could do that though.

(venv) [oliverh@niwa-1007885 cylc-flow]$ git log tests/cylc-poll/16-execution-time-limit.t | head -n 20
commit fad7acc4883ea835739050ecdf59a49056d27873
Author: Hilary James Oliver <hilary.j.oliver@gmail.com>
Date:   Mon May 13 09:41:55 2019 +1200

    Python unbuffered output, in a flaky test.

commit 93617f0462e2d4e479114b903c4ff6d58305ac34
Author: Matt Shin <matthew.shin@metoffice.gov.uk>
Date:   Fri Mar 29 16:41:44 2019 +0000

    Fix job kill hold-retry logic
    
    Prevent kill+term logic from unholding a retry (hold) status.

commit 5914621bcbf51e3e11f0dd38cbafe62266c91a55
Author: Matt Shin <matthew.shin@metoffice.gov.uk>
Date:   Tue Mar 5 16:03:21 2019 +0000

    tests/cylc-poll/16 should now be more robust

Note the last one! We seem to have negated the effect of @matthewrmshin's change...

@cylc cylc deleted a comment May 19, 2019
@hjoliver
Copy link
Member Author

@oliver-sanders - I think this is finally done. @matthewrmshin still wants a custom site config location, for those that can't access /etc/ ... but I'll punt that to a new issue since it's not clear how to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cylc and virtual environments Update CYLC_DIR after setuptools Revisit CYLC_DIR use
3 participants