Skip to content

Check experiment for hotkey logging#32285

Merged
damccorm merged 2 commits intomasterfrom
users/damccorm/hotkeyExperiment
Aug 22, 2024
Merged

Check experiment for hotkey logging#32285
damccorm merged 2 commits intomasterfrom
users/damccorm/hotkeyExperiment

Conversation

@damccorm
Copy link
Copy Markdown
Contributor

The enable_hot_key_logging experiment is a valid way to turn on hotkey logging, but we don't respect it today. This respects both the service option and the experiment.


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.

@damccorm damccorm marked this pull request as ready for review August 22, 2024 17:44
@damccorm
Copy link
Copy Markdown
Contributor Author

R: @Abacn @lostluck (trying to get this in the release to address an issue folks are running into using Dataflow templates (which can plumb through the experiment, not the service option)

@github-actions
Copy link
Copy Markdown
Contributor

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

@lostluck
Copy link
Copy Markdown
Contributor

One could argue that this should be DRY'd into the isHotKeyLoggingEnabled() method instead of at each callsite, but given how much options code is generated, I'm not sure if it would be better.

@damccorm
Copy link
Copy Markdown
Contributor Author

One could argue that this should be DRY'd into the isHotKeyLoggingEnabled() method instead of at each callsite, but given how much options code is generated, I'm not sure if it would be better.

Yeah, that was my take - I think it would've been more work than it was work (given only 3 fairly rarely touched instances)

@damccorm damccorm merged commit ea882ab into master Aug 22, 2024
@damccorm damccorm deleted the users/damccorm/hotkeyExperiment branch August 22, 2024 19:04
damccorm added a commit that referenced this pull request Aug 22, 2024
* Check experiment for hotkey logging

* Spotless
damccorm added a commit that referenced this pull request Aug 22, 2024
* Check experiment for hotkey logging

* Spotless
reeba212 pushed a commit to reeba212/beam that referenced this pull request Dec 4, 2024
* Check experiment for hotkey logging

* Spotless
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.

2 participants