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

Use get_hostname instead of socket.getfqdn #24260

Merged
merged 2 commits into from
Jun 15, 2022

Conversation

jedcunningham
Copy link
Member

We allow users to configure a different function to determine the
hostname, so we should use that consistently when we need the hostname.

@jedcunningham jedcunningham added the type:bug-fix Changelog: Bug Fixes label Jun 6, 2022
@jedcunningham jedcunningham added this to the Airflow 2.3.3 milestone Jun 6, 2022
@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Jun 6, 2022
@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jun 6, 2022
@github-actions
Copy link

github-actions bot commented Jun 6, 2022

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@potiuk
Copy link
Member

potiuk commented Jun 6, 2022

Wondering . Would that be a reason why there are entries with "nul" hostname ? I was asking the question at the development channel in slack as we seem to have repeating case where "log cannot be retrieved as hostname is missing".

@jedcunningham
Copy link
Member Author

The only thing I can think of is if the worker isn't actually able to set the hostname on the TI, so it falls back to trying to get the log from the worker with "null" in that column? The log hostname side of things already used the helper, so I don't think this would help.

airflow/www/auth.py Outdated Show resolved Hide resolved
@potiuk
Copy link
Member

potiuk commented Jun 6, 2022

Also - the proposal I had - do we EVER imagine that hostname would change mid-flight for a running process? Should we maybe ... cache it :)?

@potiuk
Copy link
Member

potiuk commented Jun 6, 2022

I think the reason why sometimes, intermittently we get a null hostname is that sometimes, intermittently the getfqdn might fail with a perfectly good system otherwise. I think this happens quite often that DNS is poorly configured or overloaded in K8S and I think this might be the root cause of the "missing hostname" problems.

Remember ?

image

@uranusjr
Copy link
Member

Looks like GitHub is having some issues.

@potiuk potiuk closed this Jun 13, 2022
@potiuk potiuk reopened this Jun 13, 2022
@potiuk
Copy link
Member

potiuk commented Jun 13, 2022

Reopened - github seems to be back up

jedcunningham and others added 2 commits June 15, 2022 10:25
We allow users to configure a different function to determine the
hostname, so we should use that consistently when we need the hostname.
Co-authored-by: Kamil Breguła <mik-laj@users.noreply.github.com>
@jedcunningham jedcunningham merged commit e29543e into apache:main Jun 15, 2022
@jedcunningham jedcunningham deleted the use_get_hostname branch June 15, 2022 19:45
ephraimbuddy pushed a commit that referenced this pull request Jun 29, 2022
We allow users to configure a different function to determine the
hostname, so we should use that consistently when we need the hostname.

(cherry picked from commit e29543e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants