[#4873] feat(core): support list group #4879

LiuQhahah · 2024-09-08T03:47:36Z

What changes were proposed in this pull request?

(Please outline the changes and how this PR fixes the issue.)

Why are the changes needed?

support list group

Fix: #4873

Does this PR introduce any user-facing change?

no

How was this patch tested?

UT

jerqi · 2024-09-09T02:30:57Z

You can refer to the pull request https://github.com/apache/gravitino/pull/4055/files

jerqi · 2024-09-24T03:09:37Z

@LiuQhahah #4055 pull request about listing users have been merged. Do you have time to rework this pull request?

LiuQhahah · 2024-09-24T05:26:05Z

@LiuQhahah #4055 pull request about listing users have been merged. Do you have time to rework this pull request?

Hi @jerqi

what action need i to do ?
just close the PR?

jerqi · 2024-09-24T06:21:41Z

@LiuQhahah #4055 pull request about listing users have been merged. Do you have time to rework this pull request?

Hi @jerqi

what action need i to do ? just close the PR?

#4055 is the pull request to support to list users.
You pull request is to list groups. Two operations are very similar.
You can refer to #4055 to modify your pull request.

There are several points to modify.

You should add more uts and integration test (You can see AccessControlIT.java).
You should support listGroupNames and listGroup operations.

LiuQhahah · 2024-09-24T08:47:10Z

@LiuQhahah #4055 pull request about listing users have been merged. Do you have time to rework this pull request?

Hi @jerqi
what action need i to do ? just close the PR?

#4055 is the pull request to support to list users. You pull request is to list groups. Two operations are very similar. You can refer to #4055 to modify your pull request.

There are several points to modify.

You should add more uts and integration test (You can see AccessControlIT.java).

You should support listGroupNames and listGroup operations.

Understood, thanks for the advice!
I've been a little busy for a while, so I'll start working on this task today.

…nt-Group-supports-list-operations

…operations # Conflicts: # clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java # core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java # core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java

jerqi · 2024-09-26T02:01:03Z

@LiuQhahah Do you have time to continue this work? Because we want to release this feature in 0.6.1. The 0.6.1 will be released at the end of this month.

…operations # Conflicts: # core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java

LiuQhahah · 2024-09-26T12:52:58Z

@LiuQhahah Do you have time to continue this work? Because we want to release this feature in 0.6.1. The 0.6.1 will be released at the end of this month.

Hi @jerqi
I'll submit a version tonight.

jerqi · 2024-09-26T12:58:37Z

@LiuQhahah Do you have time to continue this work? Because we want to release this feature in 0.6.1. The 0.6.1 will be released at the end of this month.

Hi @jerqi I'll submit a version tonight.

I have updated partial user list implement. You can see #4894

jerqi · 2024-09-26T12:59:07Z

clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java

@@ -227,6 +227,15 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE
    return getMetalake().getGroup(group);
  }

+


Javadoc is necessary.

I'll add to that, thanks.

jerqi · 2024-09-26T13:52:04Z

@LiuQhahah Could you give the permission of your repo? I can help you finish this pull request. We are eager to finish the feature before 0.6.1.

LiuQhahah · 2024-09-26T13:58:54Z

@LiuQhahah Could you give the permission of your repo? I can help you finish this pull request. We are eager to finish the feature before 0.6.1.

Okay, thanks.

…ment-Group-supports-list-operations

jerryshao · 2024-09-27T08:31:35Z

clients/client-java/src/test/java/org/apache/gravitino/client/TestUserGroup.java

+            NoSuchMetalakeException.class,
+            () -> {
+              gravitinoClient.listGroupNames();
+            });


() -> gravitinoClient.listGroupNames()

jerryshao · 2024-09-27T09:03:48Z

core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java

+  String[] listGroupNames(String metalake) {
+    return Arrays.stream(listGroupInternal(metalake, false))
+        .map(Group::name)
+        .toArray(String[]::new);
+  }


Can you please move this method above, after listGroups.

yuqi1129 · 2024-09-27T09:29:03Z

...a/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java

+        + GROUP_TABLE_NAME
+        + " gt JOIN "
+        + MetalakeMetaMapper.TABLE_NAME
+        + " mt ON gt.metalake_id = mt.metalake_id  WHERE mt.metalake_name = #{metalakeName}"


excessive space before WHERE

OK, removed.

yuqi1129 · 2024-09-27T09:37:14Z

common/src/main/java/org/apache/gravitino/dto/responses/GroupListResponse.java

+        .forEach(
+            group -> {
+              Preconditions.checkArgument(
+                  StringUtils.isNotBlank(group.name()), "group 'name' must not be null and empty");


null or blank

jerryshao · 2024-09-27T11:12:14Z

@jerqi this PR cannot be backported automatically, please help to do it manually.

support list group support list group Fix: apache#4873 no UT --------- Co-authored-by: Rory <[email protected]>

### What changes were proposed in this pull request? Supports to list groups. ### Why are the changes needed? Fix: #4873 ### Does this PR introduce _any_ user-facing change? I will add the document later. ### How was this patch tested? UT. Co-authored-by: Qiang-Liu <[email protected]>

apache#4873 support list group

10070c1

LiuQhahah added 3 commits September 24, 2024 19:16

Merge remote-tracking branch 'origin/main' into apache#4873-Improveme…

7f2f3c9

…nt-Group-supports-list-operations

apache#4873 support list group

eb8cba6

LiuQhahah added 2 commits September 26, 2024 20:47

apache#4873 support list group

3614812

Merge branch 'main' into apache#4873-Improvement-Group-supports-list-…

779671e

…operations # Conflicts: # core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java

LiuQhahah added 3 commits September 26, 2024 20:56

apache#4873 support list group format

4e14f9d

apache#4873 support list group : format

a0206a9

apache#4873 support list group : format

95034a3

jerqi reviewed Sep 26, 2024

View reviewed changes

LiuQhahah added 2 commits September 26, 2024 21:13

apache#4873 support list group

ed9cc9d

apache#4873 support list group add javaDoc

2e24f4b

Merge remote-tracking branch 'upstream/main' into apache#4873-Improve…

e5dcd17

…ment-Group-supports-list-operations

jerqi added the branch-0.6 Automatically cherry-pick commit to branch-0.6 label Sep 27, 2024

jerqi assigned LiuQhahah Sep 27, 2024

jerqi added 2 commits September 27, 2024 15:05

Align to list users

81a8d83

Fix wrong use

4a85ff7

jerqi requested review from jerryshao, yuqi1129 and xunliu September 27, 2024 07:13

add comment

9df97a1

yuqi1129 changed the title ~~#4873 support list group~~ [#4873] feat(core): support list group Sep 27, 2024

jerqi added 2 commits September 27, 2024 15:27

fix comment

90d13dc

fix comment

97dc884

jerryshao reviewed Sep 27, 2024

View reviewed changes

jerqi added 2 commits September 27, 2024 17:22

address comment

9c893d8

fix style

eb35f96

yuqi1129 reviewed Sep 27, 2024

View reviewed changes

address comment

81b225f

jerryshao approved these changes Sep 27, 2024

View reviewed changes

jerryshao assigned jerqi Sep 27, 2024

jerryshao merged commit 517f66c into apache:main Sep 27, 2024
26 checks passed

jerqi added a commit to qqqttt123/gravitino that referenced this pull request Sep 27, 2024

[apache#4873] feat(core): support list group (apache#4879)

ec0a340

support list group support list group Fix: apache#4873 no UT --------- Co-authored-by: Rory <[email protected]>

LiuQhahah deleted the #4873-Improvement-Group-supports-list-operations branch September 27, 2024 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#4873] feat(core): support list group #4879

[#4873] feat(core): support list group #4879

LiuQhahah commented Sep 8, 2024

jerqi commented Sep 9, 2024

jerqi commented Sep 24, 2024 •

edited

Loading

LiuQhahah commented Sep 24, 2024

jerqi commented Sep 24, 2024 •

edited

Loading

LiuQhahah commented Sep 24, 2024

jerqi commented Sep 26, 2024

LiuQhahah commented Sep 26, 2024

jerqi commented Sep 26, 2024

jerqi Sep 26, 2024 •

edited

Loading

LiuQhahah Sep 26, 2024

jerqi commented Sep 26, 2024 •

edited

Loading

LiuQhahah commented Sep 26, 2024

jerryshao Sep 27, 2024

jerqi Sep 27, 2024

jerryshao Sep 27, 2024

jerqi Sep 27, 2024

yuqi1129 Sep 27, 2024

jerqi Sep 27, 2024

yuqi1129 Sep 27, 2024

jerqi Sep 27, 2024

jerryshao commented Sep 27, 2024

		@@ -227,6 +227,15 @@ public Group getGroup(String group) throws NoSuchGroupException, NoSuchMetalakeE
		return getMetalake().getGroup(group);
		}

[#4873] feat(core): support list group #4879

[#4873] feat(core): support list group #4879

Conversation

LiuQhahah commented Sep 8, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

jerqi commented Sep 9, 2024

jerqi commented Sep 24, 2024 • edited Loading

LiuQhahah commented Sep 24, 2024

jerqi commented Sep 24, 2024 • edited Loading

LiuQhahah commented Sep 24, 2024

jerqi commented Sep 26, 2024

LiuQhahah commented Sep 26, 2024

jerqi commented Sep 26, 2024

jerqi Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerqi commented Sep 26, 2024 • edited Loading

LiuQhahah commented Sep 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerryshao commented Sep 27, 2024

jerqi commented Sep 24, 2024 •

edited

Loading

jerqi commented Sep 24, 2024 •

edited

Loading

jerqi Sep 26, 2024 •

edited

Loading

jerqi commented Sep 26, 2024 •

edited

Loading