-
Notifications
You must be signed in to change notification settings - Fork 5.7k
feat(outputs.microsoft_fabric): Add plugin #16827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Renamed Eventhubs
Lint fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @asaharn! I have some comments in the code, but I do have a general issue with understanding the purpose of this plugin... My understanding is that this is a wrapper around the azure_data_explorer
plugin, named EventStream
here, and the not yet existing EeventHouse plugin. So why don't you simply add a azure_eventhouse
plugin instead of this plugin and get rid of all the guesswork? The user needs to know the service anyway as he/she must fill-out the config sections...
Hi @srebhan, thank you for your time. We propose using the MSFabric plugin as the output plugin, as Microsoft Fabric is positioned as a standalone product offering (distinct from Azure) and includes components like Eventhouse and Eventstream/Eventhub as part of its underlying storage components. Fabric users should not have to manage multiple plugins or complex configurations to interact with these components. Instead, they can simply provide the connection string and have data seamlessly flushed to the appropriate storage. |
@asaharn any update regarding what we discussed on Slack, i.e. to implicitly define the required parameters with the connection string and the plugin will then parse those out without an explicit, endpoint-specific configuration? |
Hi @srebhan, Apologies for the delay, I will mostly complete it by the end of this week |
@srebhan have pushed the changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update @asaharn! The code is much better now. I have some comments in the code...
Hi @srebhan, updated the changes. |
3f01e09
to
e8eb802
Compare
@asaharn I took the freedom to push some required changes. Let me know if the code is OK for you! |
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
@srebhan Thankyou. Have checked the changes, we can proceed. |
Hi @skartikey I have pushed the changes. Requesting for a re review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asaharn Great job!
Thankyou @srebhan and @skartikey |
Co-authored-by: Sven Rebhan <srebhan@influxdata.com>
Co-authored-by: Sven Rebhan <srebhan@influxdata.com>
Summary
Checklist
Related issues
resolves #16371