> On May 19, 2020, 4:26 p.m., Ashutosh Mestry wrote: > > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java > > Lines 510 (patched) > > <https://reviews.apache.org/r/72527/diff/1/?file=2232586#file2232586line510> > > > > Refactor: Extract constant.
This statement checks whether the string is enclosed in double quotes, so can't make use of constants here > On May 19, 2020, 4:26 p.m., Ashutosh Mestry wrote: > > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java > > Lines 512 (patched) > > <https://reviews.apache.org/r/72527/diff/1/?file=2232586#file2232586line512> > > > > Refactor: Promote to const field. These set of characters are specific to this method, so won't be of much use if we promote to class level field. - Jayendra ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72527/#review220830 ----------------------------------------------------------- On May 25, 2020, 9:04 a.m., Jayendra Parab wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72527/ > ----------------------------------------------------------- > > (Updated May 25, 2020, 9:04 a.m.) > > > Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath > Subramanian. > > > Bugs: ATLAS-3779 > https://issues.apache.org/jira/browse/ATLAS-3779 > > > Repository: atlas > > > Description > ------- > > The JAAS config set through InMemoryJAASConfiguration overwrites user > supplied JAAS configuration which might be required to connect to user's > Kakfa instead of Atlas's Kafka and might have different credentials > > To resolve this we should make use of Kafka's dynamic JAAS configuration > mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config > property. > (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients) > > In this patch, we are picking up the atlas.jaas.* properties and setting the > sasl.jaas.config property. Since the JAAS configuration is specific only to > Kafka, the JAAS property initialization is done while instantiating the > KafkaNotification object. Also, since we are using sasl.jaas.config property > to set the configuration it's not conflicting with any other JAAS > configuration. > > To keep the code backward compatible for client application which make use of > ticketBased-KafkaClient configuration , we are still making use of the > earlier mechanism i.e. with the use of > UserGroupInformation.isLoginTicketBased() and > UserGroupInformation.isLoginKeytabBased() to set the configuration. > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e > notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8 > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java > 11a29b9e0 > > notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java > 9b5891f27 > > > Diff: https://reviews.apache.org/r/72527/diff/2/ > > > Testing > ------- > > With these changes, tried creating table using CTAS query in Hive. The > entities corresponding to the table was updated properly in Atlas server > > > Thanks, > > Jayendra Parab > >
