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

Add Python 3.7 compatibility #2466

Merged
merged 13 commits into from
Sep 28, 2018
Merged

Conversation

dlstadther
Copy link
Collaborator

Description

Python 3.7.0 has been released. Time to check for Luigi compatibility.

Motivation and Context

Let's try to keep up with most recent Python versions!

Have you tested this? If so, how?

Letting Travis do this :)

@dlstadther
Copy link
Collaborator Author

If successful, will need to update README

@dlstadther
Copy link
Collaborator Author

At least one error is caused by: getmoto/moto#1706

Copy link
Contributor

@Tarrasch Tarrasch left a comment

Choose a reason for hiding this comment

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

Awesome!

@dlstadther
Copy link
Collaborator Author

With regard to the moto error, we'll need to update our moto version with the Python 3.7 compatible version.

We're currently testing against moto<1.0. Moto 1.0.0 denotes breaking changes for decorators.

So an additional PR may need to be submitted to update our moto test dependency. I'll submit a naive WIP one to see how badly things break.

* upstream-master:
  Remove long-deprecated scheduler config variable alternatives (spotify#2491)
  Bump tornado milestone version (spotify#2490)
  Update moto to 1.x milestone version (spotify#2471)
  Use passed password when create a redis connection (spotify#2489)
  S3 client refactor (spotify#2482)
  Rename to rpc_log_retries, and make it apply to all the logging involved
  Factor log_exceptions into a configuration parameter
  Fix attribute forwarding for tasks with dynamic dependencies (spotify#2478)
  Add a visiblity level for luigi.Parameters (spotify#2278)
  Add support for multiple requires and inherits arguments (spotify#2475)
  Add metadata columns to the RDBMS contrib (spotify#2440)
  Fix race condition in luigi.lock.acquire_for (spotify#2357) (spotify#2477)
  tests: Use RunOnceTask where possible (spotify#2476)
  Optional TOML configs support (spotify#2457)
  Added default port behaviour for Redshift (spotify#2474)
  Add codeowners file with default and specific example (spotify#2465)
  Add Data Revenue to the `blogged` list (spotify#2472)
  Fix Scheduler.add_task to overwrite accepts_messages attribute. (spotify#2469)
  Use task_id comparison in Task.__eq__. (spotify#2462)
@dlstadther dlstadther requested a review from ulzha as a code owner August 16, 2018 10:46
@dalejung
Copy link

@dlstadther

The dist/sudo flags need to set within the matrix so they don't break other python builds.

Also, I guess the previous trusty image had postgres installed/running without specification. but it looks like postgres needs to be added into services for the xenial dist.

dalejung#1 is green on Travis. Lemme know if you want me to open a new PR or just grab the changes from there.

@dlstadther
Copy link
Collaborator Author

Thanks so much @dalejung ! I hadn't found the time to resolve the postgres issue. I've copied your build changes here.

@dalejung dalejung mentioned this pull request Aug 17, 2018
@dalejung
Copy link

👍 Had to wrangle with travis updating some personal libs. Happy the info was useful elsewhere.

* upstream-master:
  Fix S3Client.copy return value consistency (spotify#2488)
  s3client check for deprecated host keyword and raise error with the details (spotify#2493)
  Fix exception when toml lib is not installed (spotify#2506)
  Add Okko to companies that use luigi (spotify#2512)
  Added optional choice for hdfs clients  (spotify#2487)
  Version 2.7.8
  revert tornado upgrade
  Version 2.7.7
  added a new event 'progress' (spotify#2498)
  Add Uppsala University / pharmb.io as user (spotify#2496)
@dlstadther dlstadther changed the title Check for Python 3.7 support Add Python 3.7 compatibility Sep 1, 2018
@dlstadther
Copy link
Collaborator Author

GREEN BUILD! Houston, we have Python 3.7 compatibility!

@dlstadther
Copy link
Collaborator Author

dlstadther commented Sep 1, 2018

@ulzha / @honnix How's this look to you?

At some point, we should probably do something about our test speed. We're now exceeding 30m to find out if everything passes. What are your (and @Tarrasch 's) thoughts on cutting down to Py2.7, 3.7 explicit support? I understand many people may be using 34/35/36, but testing nonhdfs for each gets resource hungry.

@honnix
Copy link
Member

honnix commented Sep 27, 2018

Hard decision, but 30m is way too much. I wish I could find some stats of which Python versions are more popular than the other. :/

@honnix
Copy link
Member

honnix commented Sep 27, 2018

I guess my above answer is not helping at all. :D How about we drop 34/35 first? Give 36 one more year?

@dlstadther
Copy link
Collaborator Author

I'd support dropping tests for 34/35. If we go that route, i'd suggest updating our README to denote previous support for those versions, but that they are no longer included in the the automated test suite.

@honnix
Copy link
Member

honnix commented Sep 27, 2018

Sound like a plan. Let's go for it. BTW, in readme we say 3.3 still, lol.

@dlstadther
Copy link
Collaborator Author

Cool. I push those changes here shortly. Then this will be good to merge. Thanks @honnix !

@dlstadther
Copy link
Collaborator Author

26 min build. But a step in the right direction

@honnix
Copy link
Member

honnix commented Sep 28, 2018

👍

@honnix
Copy link
Member

honnix commented Sep 28, 2018

Is this worth a minor version bump? I'm drafting and releasing 2.7.9 and this PR can go to 2.8.0 perhaps.

@dlstadther
Copy link
Collaborator Author

If we're dropping explicit (tested) support for Python versions, i'd say this deserves 2.8.0.

Thanks for taking care of 2.7.9!

@honnix
Copy link
Member

honnix commented Sep 28, 2018

:shipit:

@dlstadther dlstadther merged commit 4389332 into spotify:master Sep 28, 2018
@dlstadther dlstadther deleted the feature/py37-support branch September 28, 2018 11:52
dlstadther added a commit to dlstadther/luigi that referenced this pull request Sep 28, 2018
* upstream-master:
  Add Python 3.7 compatibility (spotify#2466)
  Refactor s3 copy into sub-methods (spotify#2508)
  Remove s3 bucket validation prior to file upload (spotify#2528)
  Version 2.7.9
  set upper bound of python-daemon
  Update MockTarget mode to accept r* or w* (spotify#2519)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants