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

Set default task comms to client server comms. #2479

Merged
merged 1 commit into from
Nov 10, 2017

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Nov 8, 2017

A few days ago, #2464 broke installation of auth files to job hosts (w/o shared FS).

@hjoliver hjoliver added the bug Something is wrong :( label Nov 8, 2017
@hjoliver hjoliver self-assigned this Nov 8, 2017
@hjoliver
Copy link
Member Author

hjoliver commented Nov 8, 2017

(It remains near impossible to effectively test remote job host interaction under all scenarios).

@@ -529,6 +529,9 @@ def get_host_item(self, item, host=None, owner=None, replace_home=False,
if owner_home is None:
owner_home = os.path.expanduser('~%s' % owner)
value = value.replace(os.environ['HOME'], owner_home)
if item == "task communication method" and value == "default":
# Translate "default" to client-server comms: "https" or "http".
value = cfg['communication']['method']
Copy link
Member Author

@hjoliver hjoliver Nov 8, 2017

Choose a reason for hiding this comment

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

(In #2464 auth files are only copied over if task comms method starts with "http" - but the value was "default"! - which refers to the general client/server comms method)

Copy link
Member Author

@hjoliver hjoliver Nov 8, 2017

Choose a reason for hiding this comment

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

I've checked the codebase and I don't see any code that relies on the value being "default".

Copy link
Contributor

Choose a reason for hiding this comment

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

I have just been bitten by this one while testing #2468.

@hjoliver
Copy link
Member Author

hjoliver commented Nov 9, 2017

Travis CI timed out around the 49 minute mark again - there were not test errors prior to that. I just restarted the build...

@matthewrmshin matthewrmshin added this to the next release milestone Nov 9, 2017
@oliver-sanders oliver-sanders merged commit 3f26659 into cylc:master Nov 10, 2017
@hjoliver hjoliver deleted the fix-job-host-auth-install branch December 4, 2017 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants