feat: Add admin APIs for AuthorizedView#2175
Merged
mutianf merged 6 commits intogoogleapis:mainfrom Mar 27, 2024
Merged
Conversation
product-auto-label
bot
added
size: xl
Change-Id: Ie31eae6da61ed0d0462e029f6247924785b239bf
trollyxia
force-pushed
the
authorized_view2
branch
from
15abda4 to
feb960c
Compare
kolea2
reviewed
Mar 19, 2024
...e-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/AuthorizedView.java
Show resolved
Hide resolved
Change-Id: If52e3a32c3259f652f1f7d34b013d1ec1fc0a773
mutianf
suggested changes
Mar 19, 2024
...e-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/AuthorizedView.java
Show resolved
Hide resolved
...e-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/AuthorizedView.java
Show resolved
Hide resolved
...d-bigtable/src/test/java/com/google/cloud/bigtable/admin/v2/it/BigtableAuthorizedViewIT.java
Show resolved
Hide resolved
...src/test/java/com/google/cloud/bigtable/admin/v2/models/UpdateAuthorizedViewRequestTest.java
Show resolved
Hide resolved
Change-Id: I2e7f04d0f7815d014928a924d4a4f26adb2b655d
mutianf
approved these changes
Mar 20, 2024
mutianf
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
mutianf
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
mutianf
added
automerge
gcf-owl-bot
bot
removed
the
owlbot:run
mutianf
removed
the
automerge
mutianf
added
do-not-merge
do not merge
igorbernstein2
requested changes
Mar 27, 2024
Contributor
igorbernstein2
left a comment
igorbernstein2
left a comment
There was a problem hiding this comment.
Looks pretty good. I think the only thing that might be missing is to update the cleanupStale rule:
| return this; | ||
| } | ||
|
|
||
| /** Configures if safety warnings should be disabled. */ |
Contributor
There was a problem hiding this comment.
nit. please expand what safety means here
Member
Author
I don't think we need to update that since when a table is deleted, all the authorized views inside that table are also cascading deleted. |
Change-Id: Iac28b6cbef3088e4e2d43d90655155369361c347
mutianf
reviewed
Mar 27, 2024
| } | ||
|
|
||
| /** Gets the list of column qualifiers included in this authorized view. */ | ||
| public ImmutableList<ByteString> getQualifiers() { |
Contributor
There was a problem hiding this comment.
Change the API to return List cause we don't want to leak guava dependencies:
public List<ByteString> getQualifiers() {
return ImmutableList.copyOf(...);
}
| } | ||
|
|
||
| /** Gets the list of column qualifier prefixes included in this authorized view. */ | ||
| public ImmutableList<ByteString> getQualifierPrefixes() { |
| } | ||
|
|
||
| /** Gets the row prefixes to be included in this subset view. */ | ||
| public ImmutableList<ByteString> getRowPrefixes() { |
| } | ||
|
|
||
| /** Gets the map from familyId to familySubsets in this subset view. */ | ||
| public ImmutableMap<String, FamilySubsets> getFamilySubsets() { |
| /** Adds a new familyId with its familySubsets to the subset view. */ | ||
| public SubsetView addFamilySubsets(String familyId, FamilySubsets familySubsets) { | ||
| if (this.builder.containsFamilySubsets(familyId)) { | ||
| // If the familyId exists, append the familySubsets to the existing value. |
Contributor
There was a problem hiding this comment.
should the default behavior be replacing instead of merging?
| .setAuthorizedViewType( | ||
| SubsetView.create() | ||
| .addRowPrefix("row#") | ||
| .addFamilySubsets("cf1", FamilySubsets.create().addQualifier("qualifier")) |
Contributor
There was a problem hiding this comment.
once we update the logic to replace, we probably also need to update this test
Change-Id: Ibdb2c8a62dc55c44059d5ec2296c57c7d430baa4
trollyxia
force-pushed
the
authorized_view2
branch
from
595cfa1 to
e28c385
Compare
mutianf
approved these changes
Mar 27, 2024
mutianf
added
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
mutianf
added
automerge
gcf-owl-bot
bot
removed
the
owlbot:run
igorbernstein2
approved these changes
Mar 27, 2024
gcf-merge-on-green
bot
removed
the
automerge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change-Id: Ie31eae6da61ed0d0462e029f6247924785b239bf