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-7223] Add ValidatesRunner for Flink for python 3.5 - 3.6 - 3.7 #8877

Closed
wants to merge 1 commit into from

Conversation

fredo838
Copy link
Contributor

@fredo838 fredo838 commented Jun 17, 2019

This is is part of a series of PRs with goal to make Apache Beam PY3 compatible. The proposal with the outlined approach has been documented here: https://s.apache.org/beam-python-3.

This PR adds ValidatesRunner tests for Flink, as well as mirroring the file structure in test-suites/dataflow


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

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.

@fredo838
Copy link
Contributor Author

R: @tvalentyn

@fredo838
Copy link
Contributor Author

Run Python PreCommit

1 similar comment
@fredo838
Copy link
Contributor Author

Run Python PreCommit

@tvalentyn
Copy link
Contributor

retest this please

@@ -0,0 +1,79 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating copies for each python versions, can we do something like

apply from: "$basePath/flink_job_server.gradle"

@fredo838
Copy link
Contributor Author

Run Python_PVR_Flink PreCommit

2 similar comments
@fredo838
Copy link
Contributor Author

Run Python_PVR_Flink PreCommit

@fredo838
Copy link
Contributor Author

Run Python_PVR_Flink PreCommit

@fredo838 fredo838 force-pushed the beam-7223 branch 19 times, most recently from f7e42f4 to 2294b04 Compare June 24, 2019 14:17
@fredo838 fredo838 force-pushed the beam-7223 branch 7 times, most recently from ae8db67 to 83df787 Compare June 25, 2019 10:32
@fredo838
Copy link
Contributor Author

Run Python_PVR_Flink PreCommit

@fredo838 fredo838 force-pushed the beam-7223 branch 13 times, most recently from 8099d13 to 3b421cc Compare July 15, 2019 07:38
@fredo838
Copy link
Contributor Author

fredo838 commented Jul 15, 2019

@tvalentyn PTAL. Some notes:

  • I tested this locally, and when I ran my tests using ./gradlew :sdks:python:flinkValidatesRunner --no-parallel the test passed fine. I think the flinkValidatesRunner test overloaded Jenkins memory, which is why the tests gave weird logs. I implemented a code block to force the tests to run sequentially, so now the tests pass.
  • I stuck to Mark's proposed Gradle structure, and I used angoenka's suggestion to collaps the code, but to make that work I had to change the location of portableWordCount to also fit into that.
  • the tests take a long time to finish. Maybe we should not run these tests on 3.5-3.6?
  • I also moved some the py2 tests to /sdks/python/test-suites/portable/py27. Was this too soon (as it's still being discussed on the dev mailing list)?

@fredo838
Copy link
Contributor Author

Run Python PostCommit

@stale
Copy link

stale bot commented Sep 13, 2019

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.

@stale stale bot added the stale label Sep 13, 2019
@stale
Copy link

stale bot commented Sep 20, 2019

This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

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.

None yet

3 participants