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

[BEAM-5878] update dill min version to 0.3.1.1 and add test for functions with Keyword-only arguments #9686

Conversation

lazylynx
Copy link
Contributor

@lazylynx lazylynx commented Sep 29, 2019

update dill minimum version to 0.3.1.1 in setup.py and re-add unittest for keyword-only arguments from #9237


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
--- Build Status
Build Status
Build Status --- --- Build Status
XLang --- --- --- Build Status --- --- ---

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@lazylynx
Copy link
Contributor Author

Run Python PreCommit

@lazylynx lazylynx changed the title [BEAM-5878] update dill min version to 0.3.1.1 and add test for functions with Keyword-only arguments [WIP][BEAM-5878] update dill min version to 0.3.1.1 and add test for functions with Keyword-only arguments Sep 30, 2019
@lazylynx
Copy link
Contributor Author

lazylynx commented Oct 1, 2019

Run Python PreCommit

@lazylynx lazylynx closed this Oct 1, 2019
@tvalentyn tvalentyn reopened this Oct 31, 2019
@tvalentyn
Copy link
Contributor

Hi @lazylynx , could you please give this PR another try after resolving conflicts? We should have fixed the test failures now.

@lazylynx
Copy link
Contributor Author

lazylynx commented Nov 1, 2019

@tvalentyn Sure. Wait for a while.

@lazylynx lazylynx force-pushed the support_keyword-only-arguments_with_latest_dill branch 2 times, most recently from e58f715 to 14e1d7a Compare November 4, 2019 13:17
@@ -106,8 +106,7 @@ def get_version():
'avro>=1.8.1,<2.0.0; python_version < "3.0"',
'avro-python3>=1.8.1,<2.0.0; python_version >= "3.0"',
'crcmod>=1.7,<2.0',
# Dill doesn't guarantee comatibility between releases within minor version.
'dill>=0.3.0,<0.3.1',
'dill>=0.3.1.1,<0.4.0',
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's restrict this to < 0.3.2 due to possible backwards-incompatibility issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please keep the comment, and link uqfoundation/dill#347 in the comment. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. PTAL.

@lazylynx lazylynx force-pushed the support_keyword-only-arguments_with_latest_dill branch from 14e1d7a to 1142fb3 Compare November 5, 2019 15:07
@lazylynx lazylynx force-pushed the support_keyword-only-arguments_with_latest_dill branch from 1142fb3 to 0834b41 Compare November 5, 2019 16:50
@lazylynx lazylynx changed the title [WIP][BEAM-5878] update dill min version to 0.3.1.1 and add test for functions with Keyword-only arguments [BEAM-5878] update dill min version to 0.3.1.1 and add test for functions with Keyword-only arguments Nov 6, 2019
@tvalentyn
Copy link
Contributor

Sorry, I referenced a wrong Dill issue in my last comment, I meant to link uqfoundation/dill#341. I'll add a commit to this branch to fix this.

@tvalentyn
Copy link
Contributor

Merging since the tests passed on 0834b41, and second commit does not affect any tests.

@tvalentyn tvalentyn merged commit d8e94bb into apache:master Nov 8, 2019
@lazylynx lazylynx deleted the support_keyword-only-arguments_with_latest_dill branch February 13, 2020 15:41
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.

None yet

2 participants