You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by GitBox <gi...@apache.org> on 2021/01/25 13:43:49 UTC
[GitHub] [atlas] bmaidics opened a new pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
bmaidics opened a new pull request #118:
URL: https://github.com/apache/atlas/pull/118
Currently, the replicationFactor is missing from kafka topic entities in Atlas. We should add them.
----------------------------------------------------------------
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] [atlas] bmaidics commented on pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
Posted by GitBox <gi...@apache.org>.
bmaidics commented on pull request #118:
URL: https://github.com/apache/atlas/pull/118#issuecomment-769183849
Thanks @sarathsubramanian for the review, I updated the PR with the changes.
----------------------------------------------------------------
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] [atlas] sarathsubramanian commented on a change in pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
Posted by GitBox <gi...@apache.org>.
sarathsubramanian commented on a change in pull request #118:
URL: https://github.com/apache/atlas/pull/118#discussion_r564554312
##########
File path: addons/models/1000-Hadoop/patches/018-kafka_topic_add_rf_attribute.json
##########
@@ -0,0 +1,23 @@
+{
+ "patches": [
+ {
+ "id": "TYPEDEF_PATCH_1000_018_001",
+ "description": "Add 'replicationFactor' attribute to kafka_topic",
+ "action": "ADD_ATTRIBUTE",
+ "typeName": "kafka_topic",
+ "applyToVersion": "1.7",
+ "updateToVersion": "1.8",
+ "params": null,
+ "attributeDefs": [
+ {
+ "name": "replicationFactor",
Review comment:
can you also add this new attribute to the base 'kafka_topic' model - 1070-kafka_model.json?
----------------------------------------------------------------
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] [atlas] sarathsubramanian commented on a change in pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
Posted by GitBox <gi...@apache.org>.
sarathsubramanian commented on a change in pull request #118:
URL: https://github.com/apache/atlas/pull/118#discussion_r564562125
##########
File path: common/src/main/java/org/apache/atlas/utils/KafkaUtils.java
##########
@@ -126,15 +126,7 @@ public Integer getPartitionCount(String topicName) throws ExecutionException, In
LOG.debug("==> KafkaUtils.getPartitionCount({})", topicName);
}
- Integer partitionCount = null;
- DescribeTopicsResult describeTopicsResult = adminClient.describeTopics(Collections.singleton(topicName));
- Map<String, KafkaFuture<TopicDescription>> futureMap = describeTopicsResult.values();
- for(Map.Entry<String, KafkaFuture<TopicDescription>> futureEntry : futureMap.entrySet()) {
- KafkaFuture<TopicDescription> topicDescriptionFuture = futureEntry.getValue();
- TopicDescription topicDescription = topicDescriptionFuture.get();
- List<TopicPartitionInfo> partitionList = topicDescription.partitions();
- partitionCount = partitionList.size();
- }
+ Integer partitionCount = getPartitionList(topicName).size();
Review comment:
getPartitionList(topicName) can possibly return null. consider adding null check
----------------------------------------------------------------
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] [atlas] sarathsubramanian commented on a change in pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
Posted by GitBox <gi...@apache.org>.
sarathsubramanian commented on a change in pull request #118:
URL: https://github.com/apache/atlas/pull/118#discussion_r564562698
##########
File path: common/src/main/java/org/apache/atlas/utils/KafkaUtils.java
##########
@@ -143,6 +135,33 @@ public Integer getPartitionCount(String topicName) throws ExecutionException, In
return partitionCount;
}
+ public Integer getReplicationFactor(String topicName) throws ExecutionException, InterruptedException {
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("==> KafkaUtils.getReplicationFactor({})", topicName);
+ }
+
+ Integer replicationFactor = getPartitionList(topicName).stream().mapToInt(x -> x.replicas().size()).max().getAsInt();
Review comment:
consider adding null check for getPartitionList(topicName)
##########
File path: common/src/main/java/org/apache/atlas/utils/KafkaUtils.java
##########
@@ -143,6 +135,33 @@ public Integer getPartitionCount(String topicName) throws ExecutionException, In
return partitionCount;
}
+ public Integer getReplicationFactor(String topicName) throws ExecutionException, InterruptedException {
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("==> KafkaUtils.getReplicationFactor({})", topicName);
+ }
+
+ Integer replicationFactor = getPartitionList(topicName).stream().mapToInt(x -> x.replicas().size()).max().getAsInt();
+
+ if(LOG.isDebugEnabled()) {
+ LOG.debug("<== KafkaUtils.getReplicationFactor returning for topic {} with replicationFactor {}", topicName, replicationFactor);
+ }
+
+ return replicationFactor;
+ }
+
+ private List<TopicPartitionInfo> getPartitionList(String topicName) throws ExecutionException, InterruptedException {
+ List<TopicPartitionInfo> partitionList = null;
+
+ DescribeTopicsResult describeTopicsResult = adminClient.describeTopics(Collections.singleton(topicName));
+ Map<String, KafkaFuture<TopicDescription>> futureMap = describeTopicsResult.values();
Review comment:
can 'describeTopicsResult' be null if no topics exist? add necessary null check
----------------------------------------------------------------
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] [atlas] sarathsubramanian commented on pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
Posted by GitBox <gi...@apache.org>.
sarathsubramanian commented on pull request #118:
URL: https://github.com/apache/atlas/pull/118#issuecomment-769304721
please close the PR.
----------------------------------------------------------------
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] [atlas] bmaidics closed pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
Posted by GitBox <gi...@apache.org>.
bmaidics closed pull request #118:
URL: https://github.com/apache/atlas/pull/118
----------------------------------------------------------------
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] [atlas] sarathsubramanian commented on pull request #118: ATLAS-4111: Add replicationFactor attribute to kafka topic entity
Posted by GitBox <gi...@apache.org>.
sarathsubramanian commented on pull request #118:
URL: https://github.com/apache/atlas/pull/118#issuecomment-769304379
changes looks good. I merged the PR.
----------------------------------------------------------------
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