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

Undo changes to BeamModulePlugin.groovy #28356

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

damccorm
Copy link
Contributor

@damccorm damccorm commented Sep 7, 2023

These were added in #28255 but it looks like the assumption that these would function normally was baked deeper than we were aware of.


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 build label Sep 7, 2023
@damccorm
Copy link
Contributor Author

damccorm commented Sep 7, 2023

Run Java_Pulsar_IO_Direct PreCommit

@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #28356 (63890b9) into master (72e6b9a) will decrease coverage by 0.01%.
Report is 3 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #28356      +/-   ##
==========================================
- Coverage   72.34%   72.34%   -0.01%     
==========================================
  Files         681      681              
  Lines      100354   100357       +3     
==========================================
  Hits        72603    72603              
- Misses      26171    26174       +3     
  Partials     1580     1580              
Flag Coverage Δ
python 82.89% <ø> (-0.01%) ⬇️

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

see 7 files with indirect coverage changes

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

📢 Have feedback on the report? Share it here.

@damccorm
Copy link
Contributor Author

damccorm commented Sep 7, 2023

@volatilemolotov this actually works and the snapshot publishes correctly - https://github.com/apache/beam/actions/runs/6112953970/job/16591456917 publishes the most recent one on Sept 7. For example https://repository.apache.org/content/repositories/snapshots/org/apache/beam/beam-sdks-java-transform-service-launcher/2.51.0-SNAPSHOT/

I guess the gradle publish job already automatically appends a unique timestamp

@damccorm damccorm marked this pull request as ready for review September 7, 2023 18:14
@damccorm
Copy link
Contributor Author

damccorm commented Sep 7, 2023

R: @Abacn @volatilemolotov

@damccorm damccorm mentioned this pull request Sep 7, 2023
3 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

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

@volatilemolotov
Copy link
Contributor

I guess the gradle publish job already automatically appends a unique timestamp
This was not clear to me unfortunately. I guess it was deeper somewhere.

If the versioning is correct now then lets merge this

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.

LGTM, thanks!

@Abacn
Copy link
Contributor

Abacn commented Sep 7, 2023

Run Java PreCommit

@Abacn
Copy link
Contributor

Abacn commented Sep 7, 2023

Just notice that this PR does not trigger Java PreCommit GitHub Action. We'll need to add beammoduleplugin to its triggering path

@damccorm
Copy link
Contributor Author

damccorm commented Sep 7, 2023

Run Java_GCP_IO_Direct PreCommit

@damccorm
Copy link
Contributor Author

damccorm commented Sep 7, 2023

Run Java_Spark3_Versions PreCommit

@damccorm
Copy link
Contributor Author

damccorm commented Sep 7, 2023

Run Python_Runners PreCommit

@damccorm
Copy link
Contributor Author

damccorm commented Sep 7, 2023

Run Java PreCommit

@damccorm damccorm merged commit 5bf6fbf into master Sep 7, 2023
@damccorm damccorm deleted the users/damccorm/nightlySnapshot branch September 7, 2023 20:17
@kennknowles
Copy link
Member

This is normal "maven dependency" behavior by design. "SNAPSHOT" is a magic keyword that turns into a date stamp when publishing.

@kennknowles
Copy link
Member

It works at both ends: when a client depends on a SNAPSHOT version, it will grab the latest datestamped version available.

@volatilemolotov
Copy link
Contributor

Was not aware of that behavior, thanks for clarification

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.

4 participants