You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by "zeroflag (via GitHub)" <gi...@apache.org> on 2023/04/21 09:30:12 UTC

[GitHub] [knox] zeroflag opened a new pull request, #750: KNOX-2901 - Deleting a descriptor/provider via hadoop xml resource

zeroflag opened a new pull request, #750:
URL: https://github.com/apache/knox/pull/750

   ## What changes were proposed in this pull request?
   
   pending
   
   ## How was this patch tested?
   
   pending


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [knox] smolnar82 commented on a diff in pull request #750: KNOX-2901 - Deleting a descriptor/provider via hadoop xml resource

Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #750:
URL: https://github.com/apache/knox/pull/750#discussion_r1176307405


##########
gateway-topology-hadoop-xml/src/main/java/org/apache/knox/gateway/topology/hadoop/xml/HadoopXmlResourceParser.java:
##########
@@ -70,6 +71,7 @@ public class HadoopXmlResourceParser implements AdvancedServiceDiscoveryConfigCh
   private static final String CONFIG_NAME_APPLICATION_PREFIX = "app";
   private static final String CONFIG_NAME_SERVICE_URL = "url";
   private static final String CONFIG_NAME_SERVICE_VERSION = "version";
+  public static final String REMOVE = "remove";

Review Comment:
   We already have a constant for `remove` (see `CONFIG_NAME_PROVIDER_CONFIGS_PARAM_REMOVE` above). We may remove that one and use this new, more generic, contstant name.



##########
gateway-topology-hadoop-xml/src/main/java/org/apache/knox/gateway/topology/hadoop/xml/HadoopXmlResourceParser.java:
##########
@@ -70,6 +71,7 @@ public class HadoopXmlResourceParser implements AdvancedServiceDiscoveryConfigCh
   private static final String CONFIG_NAME_APPLICATION_PREFIX = "app";
   private static final String CONFIG_NAME_SERVICE_URL = "url";
   private static final String CONFIG_NAME_SERVICE_VERSION = "version";
+  public static final String REMOVE = "remove";

Review Comment:
   We already have a constant for `remove` (see `CONFIG_NAME_PROVIDER_CONFIGS_PARAM_REMOVE` above). We may remove that one and use this new, more generic, constant 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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [knox] zeroflag merged pull request #750: KNOX-2901 - Deleting a descriptor/provider via hadoop xml resource

Posted by "zeroflag (via GitHub)" <gi...@apache.org>.
zeroflag merged PR #750:
URL: https://github.com/apache/knox/pull/750


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [knox] smolnar82 commented on a diff in pull request #750: KNOX-2901 - Deleting a descriptor/provider via hadoop xml resource

Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #750:
URL: https://github.com/apache/knox/pull/750#discussion_r1176307405


##########
gateway-topology-hadoop-xml/src/main/java/org/apache/knox/gateway/topology/hadoop/xml/HadoopXmlResourceParser.java:
##########
@@ -70,6 +71,7 @@ public class HadoopXmlResourceParser implements AdvancedServiceDiscoveryConfigCh
   private static final String CONFIG_NAME_APPLICATION_PREFIX = "app";
   private static final String CONFIG_NAME_SERVICE_URL = "url";
   private static final String CONFIG_NAME_SERVICE_VERSION = "version";
+  public static final String REMOVE = "remove";

Review Comment:
   We already have a constant for `remove` (see `CONFIG_NAME_PROVIDER_CONFIGS_PARAM_REMOVE` above). We may remove that one and use this new, more generic, contact 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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [knox] smolnar82 commented on a diff in pull request #750: KNOX-2901 - Deleting a descriptor/provider via hadoop xml resource

Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #750:
URL: https://github.com/apache/knox/pull/750#discussion_r1176212466


##########
gateway-topology-hadoop-xml/src/main/java/org/apache/knox/gateway/topology/hadoop/xml/HadoopXmlResourceMonitor.java:
##########
@@ -97,6 +100,19 @@ private void processClouderaManagerDescriptor(String descriptorFilePath, String
     final HadoopXmlResourceParserResult result = hadoopXmlResourceParser.parse(descriptorFilePath, topologyName);
     processSharedProviders(result);
     processDescriptors(result);
+    processDeleted(descriptorsDir, result.getDeletedDescriptors(), ".json");
+    processDeleted(topologiesDir, result.getDeletedDescriptors(), ".xml");

Review Comment:
   As discussed offline, we may not need to remove topologies manually, because there are supposed to be deleted by [DescriptorsMonitor.onFileDelete](https://github.com/smolnar82/knox/blob/master/gateway-server/src/main/java/org/apache/knox/gateway/services/topology/monitor/DescriptorsMonitor.java#L67-L91).



-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [knox] zeroflag commented on a diff in pull request #750: KNOX-2901 - Deleting a descriptor/provider via hadoop xml resource

Posted by "zeroflag (via GitHub)" <gi...@apache.org>.
zeroflag commented on code in PR #750:
URL: https://github.com/apache/knox/pull/750#discussion_r1176260086


##########
gateway-topology-hadoop-xml/src/main/java/org/apache/knox/gateway/topology/hadoop/xml/HadoopXmlResourceMonitor.java:
##########
@@ -97,6 +100,19 @@ private void processClouderaManagerDescriptor(String descriptorFilePath, String
     final HadoopXmlResourceParserResult result = hadoopXmlResourceParser.parse(descriptorFilePath, topologyName);
     processSharedProviders(result);
     processDescriptors(result);
+    processDeleted(descriptorsDir, result.getDeletedDescriptors(), ".json");
+    processDeleted(topologiesDir, result.getDeletedDescriptors(), ".xml");

Review Comment:
   I checked it again, and the topology was indeed deleted from the file system. I'll remove the explicit `processDeleted` on the xml topology.
   
   ```2023-04-25 11:31:51,989  INFO  knox.gateway (HadoopXmlResourceParser.java:logParserResult(129)) - Found deleted descriptors deleted in /Users/attilamagyar/development/test/conf/descriptors/test.hxr
   2023-04-25 11:31:51,992  INFO  knox.gateway (HadoopXmlResourceMonitor.java:lambda$processDescriptors$2(143)) - Ignoring topology3 Knox descriptor update because it did not change.
   2023-04-25 11:31:53,783  INFO  knox.gateway (HadoopXmlResourceMonitor.java:processDeleted(111)) - Deleting file /Users/attilamagyar/development/test/conf/descriptors/deleted.json
   2023-04-25 11:31:55,399  INFO  knox.gateway (DescriptorsMonitor.java:onFileDelete(73)) - Deleting topology deleted.xml because the associated descriptor deleted.json was deleted.
   2023-04-25 11:31:55,888  INFO  knox.gateway (GatewayServer.java:handleDeleteDeployment(990)) - Deleting topology deleted
   2023-04-25 11:31:55,890  INFO  knox.gateway (GatewayServer.java:internalDeactivateTopology(934)) - Deactivating topology deleted
   2023-04-25 11:31:55,892  INFO  knox.gateway (GatewayServer.java:handleDeleteDeployment(997)) - Deleting deployed topology /Users/attilamagyar/development/test/data/deployments/deleted.topo.187b7c0a360
   ```



-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org