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

Fix reading strings from conf for remote logging #1284

Merged
merged 1 commit into from
Apr 3, 2016

Conversation

jlowin
Copy link
Member

@jlowin jlowin commented Apr 3, 2016

The default remote_logging values of None and False were being read as
strings rather than Python objects and therefore misinterpreted

The default remote_logging values of None and False were being read as
strings rather than Python objects and therefore misinterpreted
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 4d9f36b on jlowin:fix-remote-base-None into ab1c90b on airbnb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 67.201% when pulling 4d9f36b on jlowin:fix-remote-base-None into ab1c90b on airbnb:master.

@jlowin jlowin merged commit 31168bc into apache:master Apr 3, 2016
@jlowin jlowin deleted the fix-remote-base-None branch April 3, 2016 15:44
@bolkedebruin
Copy link
Contributor

Should this not be fixed in configuration.py itself? Eg. if "None" is found, we don't set a value? if != 'None' is bound to have some fix it by doing a "if is not None" in the future

@jlowin
Copy link
Member Author

jlowin commented Apr 3, 2016

The issue is that I had unwittingly added remote_base_log_folder = None to the airflow.cfg template, so anyone who regenerated airflow.cfg in the last month or so has a string value "None" there. It shouldn't actually be "None" or even None (the Python object); the real default is ''. So I added that line just to cover people who installed Airflow prior to this PR going into a release.

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.

4 participants