Hi channel, I have a couple of questions about ing...
# ingestion
b
Hi channel, I have a couple of questions about ingestion, and more specifically the Kafka connector. The first one is about any plans to implement statefulness in the short/medium term for this connector, in a similar way that many of the
sql
ones already do. This would be of great help for us, allowing us to remove the custom code that it is actually in charge of cleaning up stale topic metadata. The second question is actually a feature request, however I wanted to gauge a bit how useful such a feature could be for the Datahub community. This is our use case in a few words: in our Kafka deployments we model application access via internal users, these users only exist in Kafka, and it would be very useful for us if such information (internal user entities) could be also ingested (maybe optionally) by the Kafka connector. This would simplify governance and give us a more complete view on the state of the deployments. Please share your thoughts! Thanks!
b
Hi there!! There are no formal plans (yet) to support this for Kafka, but my guess is that it shouldn’t be too much effort. Would you be willing to create a feature request for this one? On the second question, we have no yet heard this request, however it should be doable if you extend the Kafka connector (or wrote your own custom extension) — how would you intend to show those users in datahub? As normal Users, like all the others im presuming? Is there a stable identifier available in Kafka that we can use to join with other user information like an email address?
b
Hi John thanks for your reply! Yes sure I can create a feature request for adding state to the Kafka connector. Regarding my second point, I gave it some thoughts and I think that it may be tricky to model this concept correctly in the current implementation. The idea is that these are local users, and that they make sense only within their domain (a Kafka cluster). It would not be possible to assign them a unique identifier because by their own nature they are not global. Therefore without a model that explicitly allows local users it may make little sense to try to ingest them. Let me know if any of my assumptions is incorrect!
regarding 1. I opened the feature request 😉 If there are no short-term plans to implement it, I could consider contributing, but I guess I’ll need some support to understand a few aspects. i.e. I was looking at how the sql stateful ingestion is tested via
smoke_tests
but I don’t find that way suitable for kafka ( I think that both mysql and kafka are there incidentally as architectural dependencies for datahub?)
b
Yesss they are. It'd be great to work with you to improve the Kafka connector! I'll nominate @helpful-optician-78938 to help consult with onboarding Kafka to to stateful ingestion. For context, Surya - Claudio is interested in making the kafka connector "stateful"
h
Hi @breezy-guitar-97226, extending stateful ingestion to any source is simple and straight-forward. If you are willing to contribute here, I would be more than happy to provide guidance here.
b
hi @helpful-optician-78938 nice to hear that! Sorry for the delay but our timezones are a a bit offset 🙂 I opened a draft pr to help the conversation: https://github.com/linkedin/datahub/pull/4028
for now my main doubts are about: • platform instance support for kafka: https://github.com/linkedin/datahub/pull/4028/files#diff-a25317878e63b8c1422f388818ab271fdc695b5d8ffc9bba00be755f4c0d4fa7R137-R141 • code duplication, given that many methods ended being very similar to the sql stateful ingestion • testing: I added smoke tests similarly to sql but ideally imho this should be tested here https://github.com/linkedin/datahub/tree/master/metadata-ingestion/tests/integration/kafka. In order to do that we may need a mock implementation of the state provider
looking forward to receiving your feedback 😉
h
Awesome job @breezy-guitar-97226! I'll review the PR and answer your questions soon.
Btw, the base PR 3807 got merged into master and you can rebase your changes on top of it
b
Hi @helpful-optician-78938, I rebased the changes as you suggested 😉 I think it is ready for a first pass!
👍 1
h
Hi @breezy-guitar-97226, very impressive work! I have added few minor comments on the PR, more around polish. Once they are addressed, we are good to merge!
b
Hi @helpful-optician-78938 thanks a lot for your code review. I followed the advice in your comments. The only point I slightly disagree is the necessity of some testing for the platform instance capability. However I tried to get the best compromise there by removing the integration test and adding a unit test for the kafka source that covers more or less the same aspects at a fraction of the maintenance cost. I hope this solution is fine with you too!
👏 1
b
Thanks for the awesome contribution Claudio!
thank you 1