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

cylc.hostuserutil: get_fqdn_by_host: include bad host name in error #2892

Merged
merged 1 commit into from
Nov 27, 2018

Conversation

matthewrmshin
Copy link
Contributor

On failure of socket.gethostbyname_ex, add bad host name to error.

Fix #2875.

@matthewrmshin matthewrmshin added this to the soon milestone Nov 27, 2018
@matthewrmshin matthewrmshin self-assigned this Nov 27, 2018
@hjoliver
Copy link
Member

@matthewrmshin - I just saw this - do you want to try to get it in 7.8.0?

@matthewrmshin
Copy link
Contributor Author

@hjoliver Up to you!

self.assertFalse(is_remote_host(None))
self.assertFalse(is_remote_host('localhost'))
self.assertFalse(is_remote_host(os.getenv('HOSTNAME')))
self.assertFalse(is_remote_host(get_host()))

def test_get_fqdn_by_host_on_bad_host(self):
"""get_fqdn_by_host bad host."""
bad_host = 'nosuchhost.nosuchdomain.org'
Copy link
Member

Choose a reason for hiding this comment

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

Damn it, I was planning to name my next computer like this.

Copy link
Member

@hjoliver hjoliver 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, tests as working.

@hjoliver
Copy link
Member

Matt, if you can get another Northern Hemisphere review on this, I'll merge it and make the release first thing in the morning (without needing to wait on Bruno).

Over 'n out 🌙

@hjoliver hjoliver modified the milestones: soon, cylc-7.8.0 Nov 27, 2018
Copy link
Member

@kinow kinow 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 to me. Same patch you mentioned in the issue, and neat solution using filename.

@kinow
Copy link
Member

kinow commented Nov 27, 2018

Approved, but will leave merge up to Hilary when he's preparing the release. Time to hit the sack now.

@hjoliver hjoliver merged commit af4bde8 into cylc:master Nov 27, 2018
@matthewrmshin matthewrmshin deleted the more-info-on-bad-host branch November 27, 2018 21:52
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.

3 participants