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

Compatibility with python3 and airflow 1.9 #17

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jamesrmccallum
Copy link

Hi,

First - thanks for this excellent library. We're using it to do some stuff that's otherwise very hard in Airflow.

We're going to be upgrading our codebase to airflow 1.9 and python 3.6.1, so I've made some changes to allow compatibility with both.

Changes as follows:

  • Make imports explicit throughout modules
  • Pass a lambda: None to the init kwargs of the PythonOperator to satisfy its check for a callable
  • Pop data_dependencies from the super().init kwargs of DiveOperatorto stop the deprecation warning that airflow emits.

I've made various edits to the unit tests since implicit bytes == str comparisons will fail under python3. Mostly this has just been passing explicit encoding='utf-8' arguments to the various as_string methods. I notice that the core library is careful to do this anyway.

All tests are passing in python2.7.13 and python3.6.1.

@lauralorenz
Copy link
Contributor

@jamesrmccallum awesome, thanks for the PR! I will look it over this holiday weekend :)

@lauralorenz
Copy link
Contributor

Weee didn't look at this at ALL back then now did we :) I did recently receive an email about 3.x compatibility so looks like there is still interest in this.

@jamesrmccallum
Copy link
Author

@lauralorenz - well it was working well when i did the PR. Let me know if you want me to make any tweaks! I've since moved on from where i was using it, but we ran it in production there with no issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants