Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
xunliu commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1701270570
##
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##
@@ -116,4 +123,67 @@ public static void checkRoleNamespace(Namespace namespace)
{
"Role namespace must have 3 levels, the input namespace is %s",
namespace);
}
+
+ // Install some post hooks used for ownership. The ownership will have the
all privileges
+ // of securable objects, users, groups, roles.
+ public static void prepareAuthorizationHooks(T manager, DispatcherHooks
hooks) {
+if (manager instanceof SupportsMetalakes) {
+ hooks.addPostHook(
+ "createMetalake",
Review Comment:
hi @jerqi I think using function name string comparing is too error-prone.
We can create a `SetOwner` annotation to supports, for example:
```
public class MetalakeManager implements MetalakeDispatcher {
@Override
@SetOwner
public BaseMetalake createMetalake(
NameIdentifier ident, String comment, Map properties)
throws MetalakeAlreadyExistsException {
long uid = idGenerator.nextId();
..
```
Use `SetOwner` annotation to comparing
```
if (method.isAnnotationPresent(SetOwner.class)) {
NameIdentifier identifier = (NameIdentifier) ((Object[]) args)[0];
GravitinoEnv.getInstance()
.ownershipManager()
.setOwner(
identifier,
Entity.EntityType.METALAKE,
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
xunliu commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1701270570
##
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##
@@ -116,4 +123,67 @@ public static void checkRoleNamespace(Namespace namespace)
{
"Role namespace must have 3 levels, the input namespace is %s",
namespace);
}
+
+ // Install some post hooks used for ownership. The ownership will have the
all privileges
+ // of securable objects, users, groups, roles.
+ public static void prepareAuthorizationHooks(T manager, DispatcherHooks
hooks) {
+if (manager instanceof SupportsMetalakes) {
+ hooks.addPostHook(
+ "createMetalake",
Review Comment:
hi @jerqi I think using function name string comparing is too error-prone.
We can create a `SetOwner` annotation to supports, for example:
```
public class MetalakeManager implements MetalakeDispatcher {
@Override
@SetOwner
public BaseMetalake createMetalake(
NameIdentifier ident, String comment, Map properties)
throws MetalakeAlreadyExistsException {
long uid = idGenerator.nextId();
..
```
Use `SetOwner` annotation to comparing
```
if (method.isAnnotationPresent(SetOwner.class)) {
NameIdentifier identifier = (NameIdentifier) ((Object[]) args)[0];
GravitinoEnv.getInstance()
.ownershipManager()
.setOwner(
identifier,
Entity.EntityType.METALAKE,
PrincipalUtils.getCurrentUserName(),
Owner.Type.USER);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
xunliu merged PR #4239: URL: https://github.com/apache/gravitino/pull/4239 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
xunliu commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1698197584
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
Review Comment:
I also feeling `DispatcherHookProxy` as a better name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1698213609
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
Review Comment:
Changed.
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -375,4 +385,23 @@ private void initGravitinoServerComponents() {
// Tag manager
this.tagManager = new TagManager(idGenerator, entityStore);
}
+
+ // Provides a universal entrance to install lifecycle hooks. This method
+ // focuses the logic of installing hooks.
+ // We should reuse the ability of *NormalizeDispatcher to avoid solving
Review Comment:
Changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
xunliu commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1698197584
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
Review Comment:
I also `DispatcherHookProxy` as a better name.
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -375,4 +385,23 @@ private void initGravitinoServerComponents() {
// Tag manager
this.tagManager = new TagManager(idGenerator, entityStore);
}
+
+ // Provides a universal entrance to install lifecycle hooks. This method
+ // focuses the logic of installing hooks.
+ // We should reuse the ability of *NormalizeDispatcher to avoid solving
Review Comment:
I think better change to `We should reuse the ability of
(Metalake|Schema|Table|Fileset|...)NormalizeDispatcher to avoid solving`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1697826874
##
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##
@@ -116,4 +123,65 @@ public static void checkRoleNamespace(Namespace namespace)
{
"Role namespace must have 3 levels, the input namespace is %s",
namespace);
}
+
+ public static void prepareAuthorizationHooks(T manager, LifecycleHooks
hooks) {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1697823423
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -317,27 +321,29 @@ private void initGravitinoServerComponents() {
this.idGenerator = new RandomIdGenerator();
// Create and initialize metalake related modules
-MetalakeManager metalakeManager = new MetalakeManager(entityStore,
idGenerator);
+MetalakeDispatcher metalakeManager = new MetalakeManager(entityStore,
idGenerator);
MetalakeNormalizeDispatcher metalakeNormalizeDispatcher =
-new MetalakeNormalizeDispatcher(metalakeManager);
+new
MetalakeNormalizeDispatcher(installLifecycleHooks(metalakeManager));
this.metalakeDispatcher = new MetalakeEventDispatcher(eventBus,
metalakeNormalizeDispatcher);
// Create and initialize Catalog related modules
this.catalogManager = new CatalogManager(config, entityStore, idGenerator);
CatalogNormalizeDispatcher catalogNormalizeDispatcher =
-new CatalogNormalizeDispatcher(catalogManager);
+new
CatalogNormalizeDispatcher(installLifecycleHooks((CatalogDispatcher)
catalogManager));
this.catalogDispatcher = new CatalogEventDispatcher(eventBus,
catalogNormalizeDispatcher);
SchemaOperationDispatcher schemaOperationDispatcher =
new SchemaOperationDispatcher(catalogManager, entityStore,
idGenerator);
SchemaNormalizeDispatcher schemaNormalizeDispatcher =
-new SchemaNormalizeDispatcher(schemaOperationDispatcher,
catalogManager);
+new SchemaNormalizeDispatcher(
+installLifecycleHooks((SchemaDispatcher)
schemaOperationDispatcher), catalogManager);
this.schemaDispatcher = new SchemaEventDispatcher(eventBus,
schemaNormalizeDispatcher);
TableOperationDispatcher tableOperationDispatcher =
new TableOperationDispatcher(catalogManager, entityStore, idGenerator);
TableNormalizeDispatcher tableNormalizeDispatcher =
-new TableNormalizeDispatcher(tableOperationDispatcher, catalogManager);
+new TableNormalizeDispatcher(
+installLifecycleHooks((TableDispatcher) tableOperationDispatcher),
catalogManager);
this.tableDispatcher = new TableEventDispatcher(eventBus,
tableNormalizeDispatcher);
PartitionOperationDispatcher partitionOperationDispatcher =
Review Comment:
OK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1697802067
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
Review Comment:
Em.. all is ok for me. But many system use life cycle hook when they use
similar mechinansim like K8S, Android.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1697802067
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
Review Comment:
Em.. all is ok for me. But many system use life cycle hook when they use
similar mechinansim like K8S, Android.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1697801175
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
+ private final LifecycleHooks hooks;
+ private final T dispatcher;
+
+ LifecycleHookProxy(T dispatcher, LifecycleHooks hooks) {
+this.hooks = hooks;
+this.dispatcher = dispatcher;
+ }
+
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args) throws
Throwable {
+Object result = method.invoke(dispatcher, args);
+List postHooks = hooks.getPostHooks(method.getName());
+for (BiConsumer hook : postHooks) {
Review Comment:
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1697800985
##
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##
@@ -116,4 +123,65 @@ public static void checkRoleNamespace(Namespace namespace)
{
"Role namespace must have 3 levels, the input namespace is %s",
namespace);
}
+
+ public static void prepareAuthorizationHooks(T manager, LifecycleHooks
hooks) {
+if (manager instanceof SupportsMetalakes) {
Review Comment:
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
yuqi1129 commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1697785428
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
Review Comment:
Perhaps it's better to name the class `DispatcherHookProxy`
##
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##
@@ -116,4 +123,65 @@ public static void checkRoleNamespace(Namespace namespace)
{
"Role namespace must have 3 levels, the input namespace is %s",
namespace);
}
+
+ public static void prepareAuthorizationHooks(T manager, LifecycleHooks
hooks) {
+if (manager instanceof SupportsMetalakes) {
Review Comment:
This method only hooks the event of `create`?
##
core/src/main/java/org/apache/gravitino/lifecycle/LifecycleHookProxy.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lifecycle;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.BiConsumer;
+
+class LifecycleHookProxy implements InvocationHandler {
+ private final LifecycleHooks hooks;
+ private final T dispatcher;
+
+ LifecycleHookProxy(T dispatcher, LifecycleHooks hooks) {
+this.hooks = hooks;
+this.dispatcher = dispatcher;
+ }
+
+ @Override
+ public Object invoke(Object proxy, Method method, Object[] args) throws
Throwable {
+Object result = method.invoke(dispatcher, args);
+List postHooks = hooks.getPostHooks(method.getName());
+for (BiConsumer hook : postHooks) {
Review Comment:
What if anyone fails halfway through?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1696413803
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -375,4 +385,23 @@ private void initGravitinoServerComponents() {
// Tag manager
this.tagManager = new TagManager(idGenerator, entityStore);
}
+
+ // Provides a universal entrance to install lifecycle hooks. This method
+ // focuses the logic of installing hooks.
+ // We should reuse the ability of *NormalizeDispatcher to avoid solving
Review Comment:
NO. It represents TableNormalizeDispatcher,SchemaNormalizeDispatcher, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
xunliu commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1696407423
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -317,27 +321,29 @@ private void initGravitinoServerComponents() {
this.idGenerator = new RandomIdGenerator();
// Create and initialize metalake related modules
-MetalakeManager metalakeManager = new MetalakeManager(entityStore,
idGenerator);
+MetalakeDispatcher metalakeManager = new MetalakeManager(entityStore,
idGenerator);
MetalakeNormalizeDispatcher metalakeNormalizeDispatcher =
-new MetalakeNormalizeDispatcher(metalakeManager);
+new
MetalakeNormalizeDispatcher(installLifecycleHooks(metalakeManager));
this.metalakeDispatcher = new MetalakeEventDispatcher(eventBus,
metalakeNormalizeDispatcher);
// Create and initialize Catalog related modules
this.catalogManager = new CatalogManager(config, entityStore, idGenerator);
CatalogNormalizeDispatcher catalogNormalizeDispatcher =
-new CatalogNormalizeDispatcher(catalogManager);
+new
CatalogNormalizeDispatcher(installLifecycleHooks((CatalogDispatcher)
catalogManager));
this.catalogDispatcher = new CatalogEventDispatcher(eventBus,
catalogNormalizeDispatcher);
SchemaOperationDispatcher schemaOperationDispatcher =
new SchemaOperationDispatcher(catalogManager, entityStore,
idGenerator);
SchemaNormalizeDispatcher schemaNormalizeDispatcher =
-new SchemaNormalizeDispatcher(schemaOperationDispatcher,
catalogManager);
+new SchemaNormalizeDispatcher(
+installLifecycleHooks((SchemaDispatcher)
schemaOperationDispatcher), catalogManager);
this.schemaDispatcher = new SchemaEventDispatcher(eventBus,
schemaNormalizeDispatcher);
TableOperationDispatcher tableOperationDispatcher =
new TableOperationDispatcher(catalogManager, entityStore, idGenerator);
TableNormalizeDispatcher tableNormalizeDispatcher =
-new TableNormalizeDispatcher(tableOperationDispatcher, catalogManager);
+new TableNormalizeDispatcher(
+installLifecycleHooks((TableDispatcher) tableOperationDispatcher),
catalogManager);
this.tableDispatcher = new TableEventDispatcher(eventBus,
tableNormalizeDispatcher);
PartitionOperationDispatcher partitionOperationDispatcher =
Review Comment:
If we didn't add the hooks for patitionDispatcher in this PR, I think you
can add comments here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
xunliu commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1696209844
##
core/src/main/java/org/apache/gravitino/authorization/AuthorizationUtils.java:
##
@@ -116,4 +123,65 @@ public static void checkRoleNamespace(Namespace namespace)
{
"Role namespace must have 3 levels, the input namespace is %s",
namespace);
}
+
+ public static void prepareAuthorizationHooks(T manager, LifecycleHooks
hooks) {
Review Comment:
I think better to add some comments for this function.
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -375,4 +385,23 @@ private void initGravitinoServerComponents() {
// Tag manager
this.tagManager = new TagManager(idGenerator, entityStore);
}
+
+ // Provides a universal entrance to install lifecycle hooks. This method
+ // focuses the logic of installing hooks.
+ // We should reuse the ability of *NormalizeDispatcher to avoid solving
Review Comment:
Which mean is `*NormalizeDispatcher`, Typo?
`*NormalizeDispatcher*`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1696226992
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -317,27 +321,29 @@ private void initGravitinoServerComponents() {
this.idGenerator = new RandomIdGenerator();
// Create and initialize metalake related modules
-MetalakeManager metalakeManager = new MetalakeManager(entityStore,
idGenerator);
+MetalakeDispatcher metalakeManager = new MetalakeManager(entityStore,
idGenerator);
MetalakeNormalizeDispatcher metalakeNormalizeDispatcher =
-new MetalakeNormalizeDispatcher(metalakeManager);
+new
MetalakeNormalizeDispatcher(installLifecycleHooks(metalakeManager));
this.metalakeDispatcher = new MetalakeEventDispatcher(eventBus,
metalakeNormalizeDispatcher);
// Create and initialize Catalog related modules
this.catalogManager = new CatalogManager(config, entityStore, idGenerator);
CatalogNormalizeDispatcher catalogNormalizeDispatcher =
-new CatalogNormalizeDispatcher(catalogManager);
+new
CatalogNormalizeDispatcher(installLifecycleHooks((CatalogDispatcher)
catalogManager));
this.catalogDispatcher = new CatalogEventDispatcher(eventBus,
catalogNormalizeDispatcher);
SchemaOperationDispatcher schemaOperationDispatcher =
new SchemaOperationDispatcher(catalogManager, entityStore,
idGenerator);
SchemaNormalizeDispatcher schemaNormalizeDispatcher =
-new SchemaNormalizeDispatcher(schemaOperationDispatcher,
catalogManager);
+new SchemaNormalizeDispatcher(
+installLifecycleHooks((SchemaDispatcher)
schemaOperationDispatcher), catalogManager);
this.schemaDispatcher = new SchemaEventDispatcher(eventBus,
schemaNormalizeDispatcher);
TableOperationDispatcher tableOperationDispatcher =
new TableOperationDispatcher(catalogManager, entityStore, idGenerator);
TableNormalizeDispatcher tableNormalizeDispatcher =
-new TableNormalizeDispatcher(tableOperationDispatcher, catalogManager);
+new TableNormalizeDispatcher(
+installLifecycleHooks((TableDispatcher) tableOperationDispatcher),
catalogManager);
this.tableDispatcher = new TableEventDispatcher(eventBus,
tableNormalizeDispatcher);
PartitionOperationDispatcher partitionOperationDispatcher =
Review Comment:
I don't add the hooks for patitionDispatcher, because partition don't need
owner now. It's ok to add post-hooks, too. If reviewers prefer adding it, I can
add it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on code in PR #4239:
URL: https://github.com/apache/gravitino/pull/4239#discussion_r1696226992
##
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##
@@ -317,27 +321,29 @@ private void initGravitinoServerComponents() {
this.idGenerator = new RandomIdGenerator();
// Create and initialize metalake related modules
-MetalakeManager metalakeManager = new MetalakeManager(entityStore,
idGenerator);
+MetalakeDispatcher metalakeManager = new MetalakeManager(entityStore,
idGenerator);
MetalakeNormalizeDispatcher metalakeNormalizeDispatcher =
-new MetalakeNormalizeDispatcher(metalakeManager);
+new
MetalakeNormalizeDispatcher(installLifecycleHooks(metalakeManager));
this.metalakeDispatcher = new MetalakeEventDispatcher(eventBus,
metalakeNormalizeDispatcher);
// Create and initialize Catalog related modules
this.catalogManager = new CatalogManager(config, entityStore, idGenerator);
CatalogNormalizeDispatcher catalogNormalizeDispatcher =
-new CatalogNormalizeDispatcher(catalogManager);
+new
CatalogNormalizeDispatcher(installLifecycleHooks((CatalogDispatcher)
catalogManager));
this.catalogDispatcher = new CatalogEventDispatcher(eventBus,
catalogNormalizeDispatcher);
SchemaOperationDispatcher schemaOperationDispatcher =
new SchemaOperationDispatcher(catalogManager, entityStore,
idGenerator);
SchemaNormalizeDispatcher schemaNormalizeDispatcher =
-new SchemaNormalizeDispatcher(schemaOperationDispatcher,
catalogManager);
+new SchemaNormalizeDispatcher(
+installLifecycleHooks((SchemaDispatcher)
schemaOperationDispatcher), catalogManager);
this.schemaDispatcher = new SchemaEventDispatcher(eventBus,
schemaNormalizeDispatcher);
TableOperationDispatcher tableOperationDispatcher =
new TableOperationDispatcher(catalogManager, entityStore, idGenerator);
TableNormalizeDispatcher tableNormalizeDispatcher =
-new TableNormalizeDispatcher(tableOperationDispatcher, catalogManager);
+new TableNormalizeDispatcher(
+installLifecycleHooks((TableDispatcher) tableOperationDispatcher),
catalogManager);
this.tableDispatcher = new TableEventDispatcher(eventBus,
tableNormalizeDispatcher);
PartitionOperationDispatcher partitionOperationDispatcher =
Review Comment:
I don't add the hooks for patitionDispatcher, because partition don't need
owner now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on PR #4239: URL: https://github.com/apache/gravitino/pull/4239#issuecomment-2255118051 @xunliu Could you help me review this pull request? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#4236] feat(core): Supports the post-hook for the managers or dispatcher [gravitino]
jerqi commented on PR #4239: URL: https://github.com/apache/gravitino/pull/4239#issuecomment-2244645835 @jerryshao Could you take a look? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
