Skip to content
This repository has been archived by the owner on Mar 15, 2022. It is now read-only.

Azure Event Hubs Sink #73

Merged
merged 1 commit into from
Oct 24, 2019
Merged

Conversation

juan-lee
Copy link
Contributor

@juan-lee juan-lee commented May 28, 2019

Signed-off-by: Juan-Lee Pang [email protected]

This pull request adds support for an Azure Event Hubs sink. The sink is inspired by the http sync, borrowing its batching and event buffering code and behavior.

@timothysc
Copy link
Contributor

@juan-lee - could you please sign off on your commit. Also, that is an enormous vendor chain for a sink.

@juan-lee
Copy link
Contributor Author

juan-lee commented Oct 4, 2019

@timothysc I added the signed off by. How can I get re-run the check?

@johnSchnake
Copy link
Contributor

@juan-lee Only the last commit is signed; all the commits on the PR need signed.

Personally I typically squash to a single commit and then sign that. That is probably the right thing to do here since originally your PR was using dep and the vendor directory but now is using modules. You just dont need the history for that dep stuff anymore.

@juan-lee
Copy link
Contributor Author

juan-lee commented Oct 5, 2019

Thank you @johnSchnake!

@juan-lee
Copy link
Contributor Author

juan-lee commented Oct 7, 2019

@timothysc @johnSchnake is there anything else you need from me to get this PR merged?

@johnSchnake
Copy link
Contributor

Another PR just merged, I'll update this branch onto master via the web UI if you want or you can rebase again.

I wish there were less dependencies here but go mod tidy didn't find anything to cleanup and the dependencies just cascade. Not sure if there is something to do about it.

I personally don't have an azure account setup to test this, have you verified it works as expected? I can build and run the basic command (from the README) and things continue to work well. Can you provide some feedback/screenshots/logs of events working with the new eventhub?

@juan-lee
Copy link
Contributor Author

juan-lee commented Oct 7, 2019

Thanks @johnSchnake I'll take a look to see if I can trim down the dependencies. Unfortunately, It's probably all being pulled in by the azure event hubs package.

I'll go ahead and rebase and capture some screenshots/logs of the working setup. I'll keep you posted. Thanks!

@juan-lee juan-lee force-pushed the eventhub branch 2 times, most recently from f09615f to 3edc0fa Compare October 8, 2019 18:44
@juan-lee
Copy link
Contributor Author

juan-lee commented Oct 8, 2019

Here are the screenshots @johnSchnake.
Screen Shot 2019-10-08 at 11 42 18 AM
Screen Shot 2019-10-08 at 11 41 30 AM

I also added some instructions on setting up the eventhub.

Signed-off-by: Juan-Lee Pang <[email protected]>
@juan-lee
Copy link
Contributor Author

juan-lee commented Oct 9, 2019

@timothysc @johnSchnake,

I'd really like to be able to use the eventhub sdk provided by azure even though it brings in a few more dependencies than all of us would like to see because I'd rather not re-implement what they've already done.

I'd like to propose that we not block this PR with the agreement that I'll file an issue to track the reduction in dependencies and follow up with the azure-event-hubs-go project to drive to resolution.

WDYT?

@juan-lee
Copy link
Contributor Author

@timothysc @johnSchnake,

Did you get a chance to read my proposal? If this can't merge without reducing dependencies, that's fine. Just let me know. Thanks!

@johnSchnake johnSchnake merged commit 733f06f into vmware-archive:master Oct 24, 2019
@johnSchnake
Copy link
Contributor

Thanks @juan-lee , sorry for the time to get back to this. 👍

@juan-lee
Copy link
Contributor Author

Thanks @juan-lee , sorry for the time to get back to this. 👍

No worries. Thanks @johnSchnake!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants