-
Notifications
You must be signed in to change notification settings - Fork 94
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
Unify get-config #4099
Unify get-config #4099
Conversation
Remove get-site-config
Just use $HOME/cylc-run instead
a182a34
to
779f592
Compare
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.
Great, can't wait to have this on master.
cylc/flow/parsec/util.py
Outdated
# Note: default handle=sys.stdout, but explicitly setting this interferes | ||
# with pytest capsys: | ||
# https://github.com/pytest-dev/pytest/issues/1132#issuecomment-147506107 |
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 think it's just the default kwarg value that is messing with pytest, could do this:
# Note: default handle=sys.stdout, but explicitly setting this interferes | |
# with pytest capsys: | |
# https://github.com/pytest-dev/pytest/issues/1132#issuecomment-147506107 | |
if not handle: | |
handle = sys.stdout |
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.
Unfortunately not, in the issue linked in the comment, it says that redefinition of sys.stdout
is the problem
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.
This works for me:
import sys
def whatever(handle=None):
if not handle:
handle = sys.stdout
handle.write('hello world')
def test_whatever(capsys):
whatever()
assert capsys.readouterr().out == 'hello world'
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.
Oh, the same test fails for a different reason in that case. The capsys replacement for sys.stdout
is not an instance of StringIO
, but doesn't have an attribute .mode
, which causes an exception at this point
cylc-flow/cylc/flow/parsec/util.py
Lines 142 to 143 in 4b5e8a9
if not isinstance(handle, StringIO) and 'b' in handle.mode: | |
msg = msg.encode() |
Could instead do?
if hasattr(handle, 'mode') and 'b' in handle.mode:
msg = msg.encode()
Note: if this doesn't get merged in time for 8.0b0, will need to edit the changelog |
76d65bd
to
bb1323e
Compare
bb1323e
to
c4402b7
Compare
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 good to me 👍
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.
Great.
These changes close #4009
get-suite-config
andget-site-config
commands asconfig
--python
option--mark-up
option--run-mode
option--suite-owner
option--tasks
and--all-tasks
options--print-run-dir
option--print-hierarchy
optionRequirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.