Skip to content
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

Implemented Event Grid Client & Publish Events #126

Merged
merged 5 commits into from
Dec 24, 2020

Conversation

Alexei-B
Copy link

Implementation for #125

@Alexei-B
Copy link
Author

Alexei-B commented Dec 21, 2020

Replicating my comments from #125 here for completeness:

  • I'm not sure what the preferred pattern is for making HTTP requests in the SDK. I've gone off and implemented my own wrappers around http and hyper that is somewhat based on the storage code and somewhat based on the key vault code. For now, I've kept it super simple, so that that it's ideally easy to throw away if it isn't what we want.

@MindFlavor
Copy link
Contributor

Nice job! Thank you!

We are migrating towards HTTP client independence (using a very simple trait) but I think we can address it later!

Copy link
Contributor

@MindFlavor MindFlavor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! 👍 👍
I only ask to simplify the client getting rid of the str borrows.

sdk/event_grid/src/event_grid_client.rs Outdated Show resolved Hide resolved
Copy link
Author

@Alexei-B Alexei-B left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings are now owned by the EventGridClient rather than borrowed.

sdk/event_grid/src/event_grid_client.rs Outdated Show resolved Hide resolved
@Alexei-B
Copy link
Author

Nice job! Thank you!

We are migrating towards HTTP client independence (using a very simple trait) but I think we can address it later!

Great, that's awesome to hear. I've seen how the trait works in storage for example and how that can be used to make mock clients for nice unit testing and I like that approach a lot. I would have tried to replicate that, but I quickly realised that I was wondering into duplicating a lot of code between the services.

@MindFlavor
Copy link
Contributor

Thank you!

@MindFlavor MindFlavor merged commit 014f154 into Azure:master Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants