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

Fetch buffer and add it to pcoll_buffer to avoid element duplication when buffer is None #27676

Merged
merged 3 commits into from
Jul 27, 2023

Conversation

AnandInguva
Copy link
Contributor

@AnandInguva AnandInguva commented Jul 26, 2023

Initial attempt was made at #27373 to solve #25315, #23228 but it resulted in failures in google3.

I missed the step of adding the ListBuffer to pcoll_buffers and buffer_id to buffers_to_clean to clean up the buffers in the later stages. Without this, sometimes the elements in the PColl gets duplicated.

With this PR, I am running a TAP train to see if the tests in google3 passes -. TAP train passes: https://fusion2.corp.google.com/presubmit/551096614/OCL:551096614:BASE:551101442:1690371932534:fe032276/targets

Fixes: #25315
Fixes: #23228


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

  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • 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.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #27676 (39431ca) into master (6cd6410) will increase coverage by 0.02%.
Report is 28 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #27676      +/-   ##
==========================================
+ Coverage   70.93%   70.95%   +0.02%     
==========================================
  Files         861      861              
  Lines      104806   104812       +6     
==========================================
+ Hits        74341    74372      +31     
+ Misses      28907    28882      -25     
  Partials     1558     1558              
Flag Coverage Δ
python 80.05% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...eam/runners/portability/fn_api_runner/fn_runner.py 87.92% <100.00%> (+0.07%) ⬆️

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AnandInguva
Copy link
Contributor Author

R: @tvalentyn

@AnandInguva AnandInguva marked this pull request as ready for review July 26, 2023 12:04
@AnandInguva AnandInguva requested a review from pabloem July 26, 2023 12:05
@github-actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

@AnandInguva AnandInguva merged commit cf438d3 into apache:master Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants