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

[BEAM-3218] Added Quota checks for PubsubMessage in PubsubBoundedWriter #4275

Closed
wants to merge 4 commits into from

Conversation

nguyent
Copy link

@nguyent nguyent commented Dec 15, 2017

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

BEAM-3218
This PR adds a check in the PubsubBoundedWriter to ensure the size of a publish request is within the allowed quota. There's an additional change in the PR which also adds quota checks on PubsubMessage object creation, specifically ensuring a message does not:

  • have more than 100 attributes
  • attr key > 256 bytes
  • attr val > 1024 bytes
  • single message size > 10mb

based on the quotas here.

@nguyent nguyent force-pushed the BEAM-3218 branch 2 times, most recently from 0c75646 to 8926feb Compare December 16, 2017 03:19
@nguyent nguyent force-pushed the BEAM-3218 branch 4 times, most recently from 347706c to 383e7a7 Compare December 29, 2017 15:51
@aaltay
Copy link
Member

aaltay commented Jan 23, 2018

R: @reuvenlax @chamikaramj

@chamikaramj
Copy link
Contributor

Sorry about the delay. Reuven, could you review or pass to someone whose familiar with PubSubIO ?

@stale
Copy link

stale bot commented Jun 9, 2018

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 9, 2018
@nguyent
Copy link
Author

nguyent commented Jun 14, 2018

Bump, will address feedback here

@kennknowles
Copy link
Member

We have turned on autoformatting of the codebase, which causes small conflicts across the board. You can probably safely rebase and just keep your changes. Like this:

$ git rebase
... see some conflicts
$ git diff
... confirmed that the conflicts are just autoformatting
... so we can just keep our changes are do our own autoformat
$ git checkout --theirs --
$ git add -u
$ git rebase --continue
$ ./gradlew spotlessJavaApply

Please ping me if you run into any difficulty.

@nguyent nguyent force-pushed the BEAM-3218 branch 2 times, most recently from 074dd55 to f706541 Compare July 6, 2018 17:21
@nguyent
Copy link
Author

nguyent commented Aug 13, 2018

Closing in favor of BEAM-2660 #3619

@nguyent nguyent closed this Aug 13, 2018
@nguyent nguyent deleted the BEAM-3218 branch August 13, 2018 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants