You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/01/20 21:21:18 UTC
[GitHub] [knox] pzampino opened a new pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
pzampino opened a new pull request #398:
URL: https://github.com/apache/knox/pull/398
## What changes were proposed in this pull request?
In the case of some services, or specific deployments of those services, there exists the need to direct the discovery process for topology generation. One example is HA HDFS deployments, where a particular nameservice can be desired to be proxied. Another example is multiple Solr instances within the same cluster, where one should be proxied but not the other(s).
This change introduces for Cloudera Manager discovery the concept of discovery-time-only service parameters in simple descriptors. This is already supported by Amabari service discovery for the aforementioned HDFS HA scenario, and the same pattern is being applied and extended for Cloudera Manager.
Service parameters beginning with "discovery-" can be declared for supported services in simple descriptors. The actual names of the parameters are service-specific.
This is an example for the HDFS nameservice selection:
`"services": [
{
"name": "WEBHDFS",
"params": {
"discovery-nameservice": "ns2"
}
},`
## How was this patch tested?
Manual testing, augmented/modified unit tests for the affected ServiceModelGenerator implementations.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #398:
URL: https://github.com/apache/knox/pull/398#discussion_r561938097
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
I'm wondering if `qualifyingServiceParameters` would be a better name for this field?
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
Other than that it looks good to me. Will you submit a new PS?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino commented on a change in pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #398:
URL: https://github.com/apache/knox/pull/398#discussion_r561968345
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
New PS in progress...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #398:
URL: https://github.com/apache/knox/pull/398#discussion_r561938097
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
I'm wondering if `qualifyingServiceParameters` would be a better name for this field?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino commented on a change in pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #398:
URL: https://github.com/apache/knox/pull/398#discussion_r561942047
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
You're probably correct. I went back and forth on that naming choice.
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
New PS in progress...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino merged pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
pzampino merged pull request #398:
URL: https://github.com/apache/knox/pull/398
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] smolnar82 commented on a change in pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #398:
URL: https://github.com/apache/knox/pull/398#discussion_r561964942
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
Other than that it looks good to me. Will you submit a new PS?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino commented on a change in pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #398:
URL: https://github.com/apache/knox/pull/398#discussion_r561942047
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ServiceModel.java
##########
@@ -37,6 +37,9 @@
private final String roleType;
private final String serviceUrl;
+ // Metadata for the model object, which is not directly from the service or role configuration
+ private final Map<String, String> modelProperties = new ConcurrentHashMap<>();
Review comment:
You're probably correct. I went back and forth on that naming choice.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [knox] pzampino merged pull request #398: KNOX-2530 - Support qualifying service params for CM discovery control
Posted by GitBox <gi...@apache.org>.
pzampino merged pull request #398:
URL: https://github.com/apache/knox/pull/398
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org