[BEAM-10623] Add workflow to run Beam python tests on Linux/Windows/Mac platforms by TobKed · Pull Request #12452 · apache/beam

TobKed · 2020-08-03T13:08:57Z

Workflow run with python tests

Before merging

Before merging it is required to setup related secrets:

GCP_TESTING_BUCKET - beam-github-actions-tests
GCP_PROJECT_ID - apache-beam-testing
GCP_REGION - us-central1

Before merging it Service Account should have elevated permissions:

Storage Admin (roles/storage.admin)
Dataflow Admin (roles/dataflow.admin)

it was requested in https://issues.apache.org/jira/browse/INFRA-20675

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.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	Dataflow	Samza	Twister2
Go	---	---	---
Java
Python		---	---
XLang	---	---	---

Pre-Commit Tests Status (on master branch)

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

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

TobKed · 2020-08-03T13:15:19Z

R: @aaltay @tvalentyn @kamilwu @damgad @potiuk

TobKed · 2020-08-03T13:29:11Z

example run executed on my fork (all jobs):
https://github.com/TobKed/beam/actions/runs/193255595

aaltay

Looks very reasonable. Asked a few questions. I would also like @tvalentyn to review.

aaltay · 2020-08-03T23:29:15Z

Given the size of this run, maybe we can limit this to 'schedule' only. Do you know how long it takes to run this?

Current schedule is once a day as per line 22. How about we run it every 4 hours?

Also, all actions workflow run in parallel, right? so it takes roughly the same time as running a regular precommit? In such case we could consider running it on PR.

Does github impose any limits on how many workflow could run in parallel?

Full workflow run takes approximately 30min. check on PR ~15 minutes.
IMHO It is not short, but also not too long.
If you thinks it is still too long, maybe we could keep schedule and push to master/release branch events only? WDYT?

Does github impose any limits on how many workflow could run in parallel?
Limits for concurrent jobs are between 20-180 depends on the plan (docs).
I heard that apache has Enterprise plan fot gh-actions but I am not sure about it. Do you have more information about it @gmcdonald ?

aaltay · 2020-08-03T23:31:21Z

does sdist require wheel? If sdist contains binaries, are they compatible with other python version?

I checked it and sdist does not require wheel. Source distribution does not contain binaries and is not python/platform specific. I simplified both this job and passing source dist to dataflow job.

aaltay · 2020-08-03T23:35:24Z

@kevingg - assigned this jira to you.

Acked. It's a duplicate of BEAM-10514. The reason is the file system in test has a temp directory with a very long prefix, causing the file path exceeding windows default 260 character limit.
A normal windows temp directory prefix: c:\windows\temp\,
In the test: D:\a\beam\beam\sdks\python\target\.tox\py35-win\tmp\.
The length diff breaks the limit since usually Interactive Beam uses about 240 characters in path length.

Thanks @kevingg . Should I put BEAM-10514 here or keep BEAM-10627?

aaltay · 2020-08-03T23:35:40Z

@tvalentyn - - assigned this jira to you.

Took a look and replied on the bug. It may be an issue in GRPC or the networking configuration of the Windows host that Github uses. I do not see this error in the test suite that we run internally (on Py2.7).
@TobKed do you have a recommendation how Beam Devs could recreate the same environment as used by GitHub actions for the purposes of debugging a test or run a single test via actions? It would be good to document this in our Actions users guide.
The goal is to minimize the feedback loop as much as possible.
Feel free to reply on https://issues.apache.org/jira/browse/BEAM-10625 to keep this conversation in one place.

Sure, I added comment about specific environment in the Jira. I will add/extend documentation since #12405 is merged.

aaltay · 2020-08-03T23:35:56Z

why are we using pytest here insted of unittest? @tvalentyn

aaltay · 2020-08-03T23:36:37Z

@TheNeuralBit - - assigned this jira to you.

Ack I'll take a look. It may just take a pyarrow upgrade, which will be unblocked once we drop python 2

Pyarrow upgrade can be decoupled from dropping py2, unless we need code changes in Beam:
beam/sdks/python/setup.py
Line 157 in 5ded4ae
('pyarrow>=0.15.1,<0.18.0; python_version >= "3.0" or '

Looks like it's not actually related to pyarrow version. I think we should fix it by making the dtype explicit for the numpy arrays created here:
beam/sdks/python/apache_beam/testing/datatype_inference_test.py
Line 57 in 8d03dc6
("d", np.array([1, 2, 3])),
I have a personal Windows machine I could clone Beam on and verify, but it might be simpler to just test the fix as part of this PR.

@TheNeuralBit could take a look on 4004309 , please?
Is it the solution you mentioned?
I tested it on my local fork (run results here) and it seems to work.

Yep that's what I was suggesting!

Thanks @TheNeuralBit. I cherry-picked change to the PR.

tvalentyn · 2020-08-04T16:57:53Z

Is it possible to extract py versions into a constant on top of the file so that we have to modify them in one place when we add/remove a version?

Unfortunately it is not possible right now. At least not in convenient way, especially that in this workflow there are two two type of slightly different matrixes.

tvalentyn

Thanks, @TobKed .

General comments/questions:

Is there a way how an author or a reviewer can rerun the action to retry the test or the only way is to re-push the PR?
When we run unit tests, we create xml files with test results (see generated xml file: D:\a\beam\beam\sdks\python\pytest_py36-win.xml entries in console logs). It would be great if we could somehow visualize that to see failing tests/messages easily without having to dig through console logs. Are there any options to do that? This could arrive in future PRs.

tvalentyn · 2020-08-04T17:12:38Z

Do actions allow to extract a common subroutine and call it from other actions? For example following steps seem to be repeated several times, and perhaps could be defined in one place:
steps: - name: Checkout code uses: actions/checkout@v2 - name: Install python uses: actions/setup-python@v2 with: python-version: ${{ matrix.python }} - name: Get build dependencies working-directory: ./sdks/python run: pip install -r build-requirements.txt - name: Install requirements working-directory: ./sdks/python run: pip install setuptools --upgrade && pip install -e .

Right know it is not possible, however GitHub Actions is evolving so fast it should available soon.
Related links about upcoming related feature:
actions/runner#438
actions/runner#554

tvalentyn · 2020-08-04T17:23:08Z

What is the context of these changes?
(note: it's would be better to have them in a separate commit and tag a JIRA in that commit).
Will #12150 (comment) work?

There was problem on windows since with tempfile.NamedTemporaryFile() as dst provide tempfile which is already opened. On windows any opened file seems to be read only which coused PermissionError. I know this workaround is ugly but it the best way I found to do it.
Is separate commit with JIRA will not be lost after merging?

You can use tempfile.TemporaryDirectory() to generate a fresh temp directory and use any name as a file name within this directory with the test.

I updated it with tempfile.TemporaryDirectory(), looks much more better and intuitive now. Thanks a lot!

tvalentyn · 2020-08-04T17:47:56Z

Took a look and replied on the bug. It may be an issue in GRPC or the networking configuration of the Windows host that Github uses. I do not see this error in the test suite that we run internally (on Py2.7).
@TobKed do you have a recommendation how Beam Devs could recreate the same environment as used by GitHub actions for the purposes of debugging a test or run a single test via actions? It would be good to document this in our Actions users guide.
The goal is to minimize the feedback loop as much as possible.
Feel free to reply on https://issues.apache.org/jira/browse/BEAM-10625 to keep this conversation in one place.

tvalentyn · 2020-08-04T17:48:48Z

use os.sep

Added fix. Thanks.

I used rf'.*{os.sep}log_handler_test.py:\d+' but it caused:
.... raise source.error("bad escape %s" % escape, len(escape)) sre_constants.error: bad escape \l at position 2
https://github.com/apache/beam/pull/12452/checks?check_run_id=949595653
I reverted change.
Should I use it somehow differently?

You could use re.escape(). Not a big deal since this is a test, feel free to leave as is or simplify to r'.*log_handler_test.py:\d+'.

I had problems with re.escape() as well (escaped \d as well). For the most clean solution I just simplified it as you suggested it.

tvalentyn · 2020-08-04T17:51:09Z

Pyarrow upgrade can be decoupled from dropping py2, unless we need code changes in Beam:
beam/sdks/python/setup.py
Line 157 in 5ded4ae
('pyarrow>=0.15.1,<0.18.0; python_version >= "3.0" or '

tvalentyn · 2020-08-04T17:51:55Z

Same comment as above - this approach is not intuitive.

WDYT about adding comment in such places with JIRA ? (as you mentioned creating JIRA in #12452 (comment))

The reference to JIRA in #12452 (comment) was that you could make test changes in a separate commit such as:
[BEAM-123456] Fix tests that don't pass on Windows
and have another commit:
[BEAM-10623] Add workflow to run Beam python tests on Linux/Windows/Mac platforms.
In this case the reason behind not obvious code would be easier to infer from commit history.

tvalentyn · 2020-08-04T17:57:39Z

Also, all actions workflow run in parallel, right? so it takes roughly the same time as running a regular precommit? In such case we could consider running it on PR.

tvalentyn · 2020-08-04T17:58:30Z

Is this used anywhere?

It is unnecessary copy-paste leftover. Deleted.

TobKed · 2020-08-05T14:16:40Z

@tvalentyn answering your question:

Also, all actions workflow run in parallel, right? so it takes roughly the same time as running a regular precommit? In such case we could consider running it on PR.

Building sdk source dist is used as prerequisite for python_wordcount_dataflow job . Building source distribution is already validated for PR in .github/workflows/build_wheels.yml workflow.

TobKed · 2020-08-05T16:06:41Z

@tvalentyn thanks for review.

Is there a way how an author or a reviewer can rerun the action to retry the test or the only way is to re-push the PR?

There is an event type called workflow_dispatch which enables button to manually trigger workflow I just added it.
https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/

When we run unit tests, we create xml files with test results (see generated xml file: D:\a\beam\beam\sdks\python\pytest_py36-win.xml entries in console logs). It would be great if we could somehow visualize that to see failing tests/messages easily without having to dig through console logs. Are there any options to do that? This could arrive in future PRs.

I am not sure about visualisation, I have to investigate it more depth, however I add uploads of xml files as artifacts.

tvalentyn · 2020-08-08T01:46:29Z

@tvalentyn answering your question:
Also, all actions workflow run in parallel, right? so it takes roughly the same time as running a regular precommit? In such case we could consider running it on PR.
Building sdk source dist is used as prerequisite for python_wordcount_dataflow job . Building source distribution is already validated for PR in .github/workflows/build_wheels.yml workflow.

Not sure I follow. I meant to say: if Github Action test suites run comparative amount of time as Jenkins precommit suites, we can continue running Github Action test suites on each PR, assuming Actions Quota is not a concern. I wanted to know whether different suites from the matrix (Py3.5 on Windows, Py3.6 on MacOS) run in parallel or not.

TobKed · 2020-08-10T14:21:03Z

@tvalentyn answering your question:
Also, all actions workflow run in parallel, right? so it takes roughly the same time as running a regular precommit? In such case we could consider running it on PR.
Building sdk source dist is used as prerequisite for python_wordcount_dataflow job . Building source distribution is already validated for PR in .github/workflows/build_wheels.yml workflow.
Not sure I follow. I meant to say: if Github Action test suites run comparative amount of time as Jenkins precommit suites, we can continue running Github Action test suites on each PR, assuming Actions Quota is not a concern. I wanted to know whether different suites from the matrix (Py3.5 on Windows, Py3.6 on MacOS) run in parallel or not.

Yes, they run in parallel. It is not always the case since parallel jobs are limited to some amount, but AFAIK since apache has Enteprise plan so this number is a quite big (github actions docs - usage limits)

TobKed · 2020-08-10T19:40:07Z

I rebased on the latest master, added documentation and badges, applied TemporaryDirectory context manager for python2.7 backward tests compatibility.

tvalentyn · 2020-08-11T17:58:20Z

Suggestion: Consider using Checks that GCP variables are set or Check GCP variables throughout.

I used Check GCP variables and also changed it in workflow itself. Looks much more cleaner and simpler.

tvalentyn · 2020-08-11T17:59:11Z

nit: depend

tvalentyn · 2020-08-11T18:06:51Z

For my education, which repository we refer to here? Also, I would not hard code existing values in documentation, since the source-of-truth may be updated, and documentation may not. You could say: For example apache-beam-testing instead.

I agree. I changed it referring values as examples.

tvalentyn · 2020-08-11T18:07:40Z

nit: as [GitHub Secrets]

tvalentyn · 2020-08-11T18:20:50Z

Thanks. I forgot we are still on Py2, but very soon we can start removing Py2 support.
We actually defined this at least twice already.
beam/sdks/python/apache_beam/utils/subprocess_server_test.py
Line 39 in 0098eb6
classTemporaryDirectory:
beam/sdks/python/apache_beam/testing/test_utils.py
Line 43 in 0098eb6
classTempDir(object):
Feel free to reuse one of them and add a comment # TODO(BEAM-8371): Use tempfile.TemporaryDirectory.

Thank you. I was not aware of them. I used this first one since it returns path on enter.

tvalentyn · 2020-08-11T21:48:03Z

Thanks, @TobKed , I don't have additional comments. Once all reviewers, whose feedback you expect, give their LGTM, please squash fixup commits, and leave only the commits that you want to be added to the commit history.

TobKed · 2020-08-11T23:09:31Z

Thanks, @TobKed , I don't have additional comments. Once all reviewers, whose feedback you expect, give their LGTM, please squash fixup commits, and leave only the commits that you want to be added to the commit history.

I found I missed to use TemporaryDirectory() in sdks/python/apache_beam/typehints/typecheck_test_py3.py, I fixed it.
I also added input to manual trigger so dataflow tests have to be explicitly enabled (made them disabled by default).

Thanks for reviews @tvalentyn and all people involved in this PR. I will create PR to Apache Infra with request for setting up proper secrets.

TobKed · 2020-08-12T12:21:41Z

Run Python PreCommit

TobKed · 2020-08-12T12:21:57Z

Run Python2_PVR_Flink PreCommit

TobKed · 2020-08-12T13:40:38Z

Run Python PreCommit

TobKed · 2020-08-12T14:07:09Z

Created single PR to infra with all required changes for github secrets: https://issues.apache.org/jira/browse/INFRA-20675

tvalentyn · 2020-08-12T18:11:50Z

I also added input to manual trigger so dataflow tests have to be explicitly enabled (made them disabled by default).
Are Dataflow tests enabled in automated execution or not?

TobKed · 2020-08-12T23:51:35Z

I also added input to manual trigger so dataflow tests have to be explicitly enabled (made them disabled by default).
Are Dataflow tests enabled in automated execution or not?

Dataflow tests for automated execution are enabled as they were in previous commits (events: push/merge, scheduled). Behavior changed for manual trigger. When triggered by manual button they may be able do choose are dataflow tests shall run or not.

TobKed · 2020-08-17T18:56:12Z

I rebased on the latest master and solved conflicts.

tvalentyn · 2020-08-17T21:00:27Z

Thanks, looks like 'Upload test logs' is flaky:

 Upload test logs1s
pythonLocation: C:\hostedtoolcache\windows\Python\3.5.4\x64
Run actions/upload-artifact@v2
with:
name: pytest-windows-latest-3.5
path: sdks/python/pytest**.xml
if-no-files-found: warn
env:
pythonLocation: C:\hostedtoolcache\windows\Python\3.5.4\x64
With the provided path, there will be 2 file(s) uploaded
##[error]read ECONNRESET

Exponential backoff for retry #1. Waiting for 5006.55256840354 milliseconds before continuing the upload at offset 0
##[error]Unexpected response. Unable to upload chunk to https://pipelines.actions.githubusercontent.com/COKSSHEuHyOLM1pP93G8aBnLpu5tMJfYiveSzILlm0cIfDvSEl/_apis/resources/Containers/4328416?itemPath=pytest-ubuntu-latest-3.8%2Fpytest_py38_no_xdist.xml
##### Begin Diagnostic HTTP information #####
Status Code: 404
Status Message: Not Found
Header Information: {
"content-length": "29",
"content-type": "text/plain",
"strict-transport-security": "max-age=2592000",
"x-tfs-processid": "98f88b46-bb0d-4ece-af51-fbf9accea451",
"x-tfs-serviceerror": "The+resource+cannot+be+found.",
"x-msedge-ref": "Ref A: E85C8E28FC70417687E1D2AAE3FBDD17 Ref B: BN3EDGE0515 Ref C: 2020-08-17T18:55:44Z",
"date": "Mon, 17 Aug 2020 18:55:53 GMT"
}
###### End Diagnostic HTTP information ######
##[warning]Aborting upload for /home/runner/work/beam/beam/sdks/python/pytest_py38_no_xdist.xml due to failure
Post Checkout code

Is there a way we can configure additional retries there?

Also, how a PR author will re-run the actions tests on their PR? Is it possible to rerun only failed tests? (I remember I asked this question before, but I forgot what the answer was)

tvalentyn · 2020-08-17T21:04:03Z

Also, how a PR author will re-run the actions tests on their PR? Is it possible to rerun only failed tests? (I remember I asked this question before, but I forgot what the answer was)

Found your reply above and also found a Rerun all jobs button, which I clicked on one of the failed jobs.

TobKed · 2020-08-18T15:15:02Z

I rebased on the latest master and solved conflicts in CI.md.

@tvalentyn regarding flaky upload I tested it so many times and it never happened before. IMHO it was a temporary problem with GitHub Actions itself.
Related issue actions/upload-artifact#116

TobKed · 2020-08-18T16:47:08Z

@tvalentyn @aaltay I see that https://issues.apache.org/jira/browse/INFRA-20675 is done and just waits for confirmation.

tvalentyn · 2020-08-18T21:17:30Z

Ack, thanks. Is this ready to merge?

TobKed · 2020-08-18T21:50:20Z

@tvalentyn yes

TobKed · 2020-08-19T07:31:56Z

Thank you!

probot-autolabelerBot added build io python labels Aug 3, 2020

TobKed mentioned this pull request Aug 3, 2020
[BEAM-8665] Add test suites to run Beam tests on Linux/Windows/Mac platforms. #12318
Closed
4 tasks

aaltay reviewed Aug 3, 2020
View reviewed changes

aaltay requested a review from tvalentyn August 3, 2020 23:37

tvalentyn reviewed Aug 4, 2020
View reviewed changes

TobKed force-pushed the github-actions-test-suite-python branch from 0d41dd7 to b7883ffCompare August 5, 2020 15:30

TobKed force-pushed the github-actions-test-suite-python branch from a600b5c to 8ead399Compare August 10, 2020 18:50

TobKed requested review from aaltay and tvalentyn August 10, 2020 19:40

tvalentyn reviewed Aug 11, 2020
View reviewed changes

tvalentyn approved these changes Aug 11, 2020
View reviewed changes

TobKed force-pushed the github-actions-test-suite-python branch 3 times, most recently from 6d56be7 to e749602Compare August 11, 2020 22:58

TobKed force-pushed the github-actions-test-suite-python branch from e749602 to ad8f04cCompare August 17, 2020 18:44

Tobiasz Kędzierski added 2 commits August 18, 2020 17:10

[BEAM-10623] Add workflow to run python tests on Linux/Windows/Mac
0501123

[BEAM-10624] dtype explicit for the numpy arrays
7b19dc5

TobKed force-pushed the github-actions-test-suite-python branch from ad8f04c to 7b19dc5Compare August 18, 2020 15:11

tvalentyn merged commit c99390a into apache:masterAug 18, 2020

Uh oh!

Conversation

TobKed commented Aug 3, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before merging

Before merging it is required to setup related secrets:

Before merging it Service Account should have elevated permissions:

it was requested in https://issues.apache.org/jira/browse/INFRA-20675

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

Uh oh!

TobKed commented Aug 3, 2020

Uh oh!

TobKed commented Aug 3, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaltay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TobKedAug 7, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvalentyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TobKed commented Aug 3, 2020•
edited
Loading

TobKed commented Aug 3, 2020•
edited
Loading

TobKedAug 7, 2020•
edited
Loading

TobKedAug 5, 2020•
edited
Loading

TobKedAug 5, 2020•
edited
Loading

tvalentynAug 8, 2020•
edited
Loading