Skip to content

Conversation

@damccorm
Copy link
Contributor

This will use the following process to introduce some extra entropy into the generated key:

image

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.

@github-actions github-actions bot added the java label Nov 24, 2025
@github-actions github-actions bot added the build label Nov 25, 2025
@damccorm damccorm added this to the 2.70.0 Release milestone Nov 25, 2025
@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

❌ Patch coverage is 86.20690% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.14%. Comparing base (b20ccbf) to head (8140976).
⚠️ Report is 48 commits behind head on master.

Files with missing lines Patch % Lines
sdks/python/apache_beam/transforms/util.py 86.20% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #36891      +/-   ##
============================================
+ Coverage     55.07%   55.14%   +0.07%     
  Complexity     1667     1667              
============================================
  Files          1063     1063              
  Lines        166396   166765     +369     
  Branches       1199     1199              
============================================
+ Hits          91638    91963     +325     
- Misses        72582    72626      +44     
  Partials       2176     2176              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Amar3tto
Copy link
Collaborator

@damccorm What is the status for this?
Should we update the milestone?

@damccorm
Copy link
Contributor Author

damccorm commented Dec 1, 2025

I think we should try to get this in if we can, it is a really important patch for making sure that our GBEK story is complete, and that is probably worth delaying the release a couple of days if needed.

It still needs a review and I need to get the permissions right for GitHub Actions (this is the only thing causing suites to fail)

@damccorm damccorm marked this pull request as ready for review December 1, 2025 20:51
@damccorm
Copy link
Contributor Author

damccorm commented Dec 1, 2025

R: @Abacn since I think you reviewed pieces of this initially.

@damccorm
Copy link
Contributor Author

damccorm commented Dec 1, 2025

Feel free to ignore the failing permission causing test failures right now, that is caused by insufficiently a privileged GHA runner, in general tests are passing locally and I will address it before merging (should require no code changes)

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@Abacn Abacn self-requested a review December 1, 2025 20:55
Copy link
Contributor

@Abacn Abacn left a comment

Choose a reason for hiding this comment

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

Had a pass for Java change, also submitted one review comment for Python because triggering Python tests may need time to iterate.

Will review Python change next, or find a second eyes for Python if needed.

try:
from google.cloud import kms
except ImportError:
kms = None # type: ignore[assignment]
Copy link
Contributor

Choose a reason for hiding this comment

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

For Python SDK we have this mechanism to handle optional dependency. Just wondering if it is possible to make gcp secret dependencies optional for Java core as well. Just a side note, not needed for this PR.

Copy link
Contributor

@Abacn Abacn left a comment

Choose a reason for hiding this comment

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

Thanks. Feel free to merge after PostCommit finishing

@damccorm damccorm merged commit e3afe62 into master Dec 2, 2025
125 of 130 checks passed
@damccorm damccorm deleted the users/damccorm/kmsSecret branch December 2, 2025 21:45
damccorm added a commit that referenced this pull request Dec 2, 2025
* Add new method of generating key for GBEK

* Java version

* fix deps

* Imports

* Secret parsing tests

* docs

* more docs

* formatting + test cleanup

* lint

* lint

* lint

* lint

* import order

* Deps + style exemption

* reuse key:

* reuse key

* Feedback

* Test fixes
damccorm added a commit that referenced this pull request Dec 3, 2025
* Add new method of generating key for GBEK

* Java version

* fix deps

* Imports

* Secret parsing tests

* docs

* more docs

* formatting + test cleanup

* lint

* lint

* lint

* lint

* import order

* Deps + style exemption

* reuse key:

* reuse key

* Feedback

* Test fixes
Amar3tto added a commit that referenced this pull request Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants