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

Extend ZMQClient and update docstrings #3083

Merged
merged 2 commits into from
May 3, 2019

Conversation

kinow
Copy link
Member

@kinow kinow commented Apr 6, 2019

Tentative fix for #3014

Simply makes SuiteRuntimeClient child of ZMQClient, and call its constructor. Also updates docstrings, adds PEP-484 notation, and a TODO about a function that exits the program.

I remembered about this issue last week while working on the setup.py issue (#2989), as I noticed a couple warnings in Codacy that I think may disappear with this change too.

Travis-CI passed on my fork, but still pending Codacy inspection 👍

@kinow kinow self-assigned this Apr 6, 2019
@kinow kinow added the small label Apr 6, 2019
@kinow kinow added this to the cylc-8.0a1 milestone Apr 6, 2019
@kinow kinow force-pushed the inherit-zmqclient branch 3 times, most recently from e5de55d to 8106477 Compare April 9, 2019 01:08
try:
contact = SuiteSrvFilesManager().load_contact_file(
suite, owner, host)
except SuiteServiceFileError:
sys.exit(cls.NOT_RUNNING % suite)
Copy link
Member

Choose a reason for hiding this comment

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

This was a hangover from before #2995, thanks for fixing!

lib/cylc/network/client.py Show resolved Hide resolved
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

I'm happy with whatever you think is sensible inheritance wise. Please assign a second reviewer.

Copy link
Member

@dwsutherland dwsutherland left a comment

Choose a reason for hiding this comment

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

Looks good at a glance..

@kinow
Copy link
Member Author

kinow commented May 3, 2019

Thanks @dwsutherland , fixing the conflicted file in a bit.

@kinow
Copy link
Member Author

kinow commented May 3, 2019

Done, now Travis should be busy re-building it for a few minutes 👍

@matthewrmshin
Copy link
Contributor

(Doubly approved. Merge.)

@matthewrmshin matthewrmshin merged commit 87ab329 into cylc:master May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants