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

7.8.x patch 1 #2947

Merged
merged 8 commits into from
Mar 14, 2019
Merged

7.8.x patch 1 #2947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7dc78cc
contact file creation: fsync parent directory
matthewrmshin Feb 5, 2019
94c5dfe
host appointer: improve handling of unexpected out
matthewrmshin Feb 5, 2019
a4384c1
syntax: add reference syntax files for testing lexers
oliver-sanders Jan 8, 2019
d3a86af
syntax: feedback
sadielbartholomew Jan 16, 2019
a1b0157
Give logger a null handler on start up
matthewrmshin Feb 6, 2019
e09c270
Set location of coverage file in Travis
Jan 30, 2019
38f4151
Use absolute locations for sources
kinow Feb 1, 2019
65fff77
Fix broken links for cylc-7.8.1 change log entry
sadielbartholomew Feb 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ env:
- CYLC_TEST_RUN_PLATFORM=false
# Custom diff command to ignore Xlib errors (xvfb has not RANDR extension).
- CYLC_TEST_DIFF_CMD="diff -I Xlib -u"
- COVERAGE_PROCESS_START="${TRAVIS_BUILD_DIR}/.coveragerc"
# This coverage RC file is created under the script task
- COVERAGE_PROCESS_START="/tmp/.coveragerc"
matrix:
- CHUNK="1/4"
- CHUNK="2/4"
Expand All @@ -58,6 +59,14 @@ env:
install: .travis/install.sh functional-tests docs
script:
- export PYTHONPATH="${TRAVIS_BUILD_DIR}/.travis"
# When we run cylc commands, there are processes being forked, that get a
# new working directory. As .coveragerc contains relatives paths, it fails
# to produce the correct coverage, unless we use absolute paths. The `sed`
# call below tries to define the data_file, and sources locations for Travis.
- sed -e "s|data_file=.coverage|data_file=${TRAVIS_BUILD_DIR}/.coverage|g; s|./bin|${TRAVIS_BUILD_DIR}/bin|g; s|./lib|${TRAVIS_BUILD_DIR}/lib|g" .coveragerc > /tmp/.coveragerc
# And some tests fail if we touch files in the git working directory, due
# to Cylc's version appearing with the "dirty" suffix. To avoid this, we
# are using a new coveragerc created under the temporary directory.
- coverage run .travis/cover.py
- unset PYTHONPATH
after_script: .travis/after_script.sh
Expand Down
12 changes: 6 additions & 6 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,24 @@ Selected user-facing changes:

### Enhancements

[#2910](https://github.com/cylc/cylc/pull/#2910) - replace LaTeX-generated HTML
[#2910](https://github.com/cylc/cylc/pull/2910) - replace LaTeX-generated HTML
and PDF User Guide with Sphinx-generated HTML.

[2815](https://github.com/cylc/cylc/pull/2815) - allow initial cycle point
[#2815](https://github.com/cylc/cylc/pull/2815) - allow initial cycle point
relative to current time.

[#2902](https://github.com/cylc/cylc/pull/#2902) - expose suite UUID to event
[#2902](https://github.com/cylc/cylc/pull/2902) - expose suite UUID to event
handlers.

### Fixes

[#2932](https://github.com/cylc/cylc/pull/#2932) - fix possible blocking pipe
[#2932](https://github.com/cylc/cylc/pull/2932) - fix possible blocking pipe
due to chatty job submission (and other subprocess) commands.

[#2921](https://github.com/cylc/cylc/pull/#2921) - better suite validation
[#2921](https://github.com/cylc/cylc/pull/2921) - better suite validation
warning for out-of-bounds cycling sequences.

[#2924](https://github.com/cylc/cylc/pull/#2924) - fix and expand 7.8.0 `cylc
[#2924](https://github.com/cylc/cylc/pull/2924) - fix and expand 7.8.0 `cylc
review` documentation in the User Guide.

-------------------------------------------------------------------------------
Expand Down
25 changes: 9 additions & 16 deletions bin/cylc-cycle-point
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

"""cylc [util] cycle-point [OPTIONS] [POINT]
"""cylc [util] cycle-point [OPTIONS] ARGS

Cycle point date-time offset computation, and filename templating.

Expand Down Expand Up @@ -50,25 +50,23 @@ Other examples:
% export CYLC_TASK_CYCLE_POINT=2010-08
% export MYTEMPLATE=foo-CCYY-MM.nc
% cylc cycle-point --offset-years=2 --template=MYTEMPLATE
foo-2012-08.nc

Arguments:
[POINT] ISO 8601 date-time, e.g. 20140201T0000Z, default
$CYLC_TASK_CYCLE_POINT"""
foo-2012-08.nc"""

import os
import sys
from optparse import OptionParser

import cylc.flags
import cylc.cycling.iso8601
from cylc.option_parsers import CylcOptionParser as COP
import isodatetime.data
import isodatetime.dumpers
import isodatetime.parsers


def main():
parser = OptionParser(__doc__)
parser = COP(
__doc__,
argdoc=[
('[POINT]', 'ISO8601 date-time, default=$CYLC_TASK_CYCLE_POINT')])

parser.add_option(
"--offset-hours", metavar="HOURS",
Expand Down Expand Up @@ -132,7 +130,7 @@ def main():
"--print-hour", help="Print only hh of result",
action="store_true", default=False, dest="print_hour")

(options, args) = parser.parse_args()
options, args = parser.parse_args(remove_opts=['--host', '--user'])

if len(args) == 0:
# input cycle point must be defined in the environment.
Expand Down Expand Up @@ -266,9 +264,4 @@ def main():


if __name__ == "__main__":
try:
main()
except Exception as exc:
if cylc.flags.debug:
raise
sys.exit(str(exc))
main()
Copy link
Member

Choose a reason for hiding this comment

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

I guess I didn't review the original PR to master, but why this change? (My old "users should see an error message, not a traceback" argument is probably wrong, but we should be consistent, or remove this from all the commands, no?)

Copy link
Contributor Author

@matthewrmshin matthewrmshin Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

No, your original assumption was not wrong. However:

  • Time has changed and many users are now familiar enough with Python that the trace back is no longer scary - and it helps us and them when they send us bug reports.
  • Catching known exception (those with nice messages) here is good, but catching all generic exceptions here is not so good - especially when you have those cryptic system errors - which is much better being a trace back.

You are right. I should changed everything.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I'm happy to have this removed from all commands.

Copy link
Member

Choose a reason for hiding this comment

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

Word of warning: In Python3 when you catch an exception its context is not forgotten which results in multi-part traceback which gets really messy really quick.

For example a pattern we use a lot in Cylc is to catch a nasty OSError or KeyError or whatever and raise a more user-friendly CylcException, ConfigError or whatnot. This pattern is at Python3 fundamentally broken as you will end up raising both the original exception as well as the new one.

For example say we are trying to catch ZeroDivisionError and hide it from the user replacing it with a more informative Exception...

>>> try:
...     1/0
... except ZeroDivisionError:
...     # raise a completely different exception
...     raise Exception('foo')
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
Exception: foo

28 changes: 10 additions & 18 deletions bin/cylc-documentation
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

"""cylc [info] documentation|browse [OPTIONS] [SUITE]
"""cylc [info] documentation|browse [OPTIONS] ARGS

View documentation in the browser, as per Cylc global config.

Expand All @@ -25,11 +25,8 @@ View documentation in the browser, as per Cylc global config.

% cylc doc [-t TASK] SUITE
View suite or task documentation, if URLs are specified in the suite. This
parses the suite definition to extract the requested URL. Note that suite
server programs also hold suite URLs for access from the Cylc GUI.

Arguments:
[TARGET] File, URL, or suite name"""
parses the suite definition to extract the requested URL. Note that suite
server programs also hold suite URLs for access from the Cylc GUI."""

import sys

Expand All @@ -41,16 +38,17 @@ for arg in sys.argv[1:]:

import os
import re
import shlex
from subprocess import call
from optparse import OptionParser

import cylc.flags
from cylc.cfgspec.glbl_cfg import glbl_cfg
from cylc.option_parsers import CylcOptionParser as COP
from cylc.run_get_stdout import run_get_stdout


def main():
parser = OptionParser(__doc__)
parser = COP(
__doc__, argdoc=[('[TARGET]', 'File or suite name')])

parser.add_option(
"-g", "--guides",
Expand Down Expand Up @@ -91,8 +89,7 @@ def main():
help="URL to view in your configured browser.",
metavar="URL", default=None, action="store", dest="url")

(options, args) = parser.parse_args()
cylc.flags.debug = options.debug
options, args = parser.parse_args(remove_opts=['--host', '--user'])

intranet_url = glbl_cfg().get(['documentation', 'urls', 'local index'])
internet_url = glbl_cfg().get(['documentation', 'urls',
Expand Down Expand Up @@ -159,7 +156,7 @@ def main():

# viewer may have spaces (e.g. 'firefox --no-remote'):
command = '%s %s' % (viewer, target)
command_list = re.split(' ', command)
command_list = shlex.split(command)

if options.stdout:
print target
Expand All @@ -178,9 +175,4 @@ def main():


if __name__ == "__main__":
try:
main()
except Exception as exc:
if cylc.flags.debug:
raise
sys.exit(str(exc))
main()
28 changes: 4 additions & 24 deletions bin/cylc-get-gui-config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,13 @@ use -i/--item and wrap parent sections in square brackets:
cylc get-gui-config --item '[themes][default]succeeded'
Multiple items can be specified at once."""

import sys
from optparse import OptionParser

from cylc.cfgspec.gcylc import GcylcConfig
import cylc.flags
from cylc.option_parsers import CylcOptionParser as COP


def main():
parser = OptionParser(__doc__)

parser.add_option(
"-v", "--verbose", help="Print extra information.",
action="store_true", default=False, dest="verbose")

parser.add_option(
"--debug", help="Show exception tracebacks.",
action="store_true", default=False, dest="debug")
parser = COP(__doc__, argdoc=[])

parser.add_option(
"-i", "--item", metavar="[SEC...]ITEM",
Expand All @@ -57,22 +47,12 @@ def main():
"-p", "--python", help="Print native Python format.",
action="store_true", default=False, dest="pnative")

(options, args) = parser.parse_args()
cylc.flags.verbose = options.verbose
cylc.flags.debug = options.debug

if len(args) != 0:
parser.error("ERROR: wrong number of arguments")
options = parser.parse_args(remove_opts=['--host', '--user'])[0]

# Import gcfg here to avoid aborting before command help is printed.
GcylcConfig.get_inst().idump(
options.item, sparse=options.sparse, pnative=options.pnative)


if __name__ == "__main__":
try:
main()
except Exception as exc:
if cylc.flags.debug:
raise
sys.exit(str(exc))
main()
33 changes: 6 additions & 27 deletions bin/cylc-get-site-config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ use -i/--item and wrap parent sections in square brackets:
cylc get-site-config --item '[editors]terminal'
Multiple items can be specified at once."""

import sys
from optparse import OptionParser
import cylc.flags

from cylc.cfgspec.glbl_cfg import glbl_cfg
from cylc.option_parsers import CylcOptionParser as COP


def main():
parser = OptionParser(__doc__)
parser = COP(__doc__, argdoc=[])

parser.add_option(
"-i", "--item", metavar="[SEC...]ITEM",
Expand All @@ -58,24 +58,8 @@ def main():
help="Print the cylc site configuration directory location.",
action="store_true", default=False, dest="site_dir")

parser.add_option(
"-v", "--verbose", help="Print extra information.",
action="store_true", default=False, dest="verbose")

parser.add_option(
"--debug",
help="Show exception tracebacks.",
action="store_true", default=False, dest="debug")

(options, args) = parser.parse_args()
cylc.flags.verbose = options.verbose
cylc.flags.debug = options.debug

if len(args) != 0:
parser.error("ERROR: wrong number of arguments")
options = parser.parse_args(remove_opts=['--host', '--user'])[0]

# import glbl_cfg here to avoid aborting before command help is printed
from cylc.cfgspec.glbl_cfg import glbl_cfg
if options.run_dir:
print glbl_cfg().get_host_item('run directory')
elif options.site_dir:
Expand All @@ -86,9 +70,4 @@ def main():


if __name__ == "__main__":
try:
main()
except Exception as exc:
if cylc.flags.debug:
raise
sys.exit(str(exc))
main()
Loading