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.

Sign up for GitHub

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

Jump to bottom

Connects proton-j to service and contacts CBS node for authorization #3795

Merged
merged 14 commits into from
Jun 4, 2019
Merged

Connects proton-j to service and contacts CBS node for authorization #3795

merged 14 commits into from
Jun 4, 2019

Conversation

conniey
Copy link
Member

@conniey conniey commented Jun 1, 2019 •
edited
Loading

  • This is a huge PR because there was a lot of things needed to connect to the CBS node and get authorization working.
    • EventHubClientTest.getPartitionInformation is the single test that I have which verifies the end-to-end scenario works.

Changes

  • There is a com.azure.core.amqp in azure-eventhubs with classes I wasn't sure belonged in azure.core yet. If they don't, I'll dump them back into the implementation folder.
  • Adds a proton-j handlers that are required to listen to various events that occur
  • Adds Reactor* implementation of AmqpConnection, AmqpSession, etc.
  • Adds ShutdownSignal, ExceptionHandler, StateNotifier, to inform parents of any issues that occur in their child nodes. (rather than passing the parent's reference to the child and invoking methods on them.)
  • EventData can decode a Proton-j message

NOTE

Code is still in-flux, the RequestResponesChannel will probably go away and be merged into a sender that waits for an acknowledgement before completing.

TODO

  • Add tests

Fixes #3600

@conniey conniey added Event Hubs Client This issue points to a problem in the data-plane of the library. labels Jun 1, 2019
@conniey conniey requested review from JonathanGiles and mssfang June 1, 2019 07:51
@conniey conniey self-assigned this Jun 1, 2019
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

conniey added 4 commits June 1, 2019 21:48
@conniey
Remove types that are no longer needed
88afd68 
Moving EventDataUtil into EventData.

Add ability to deserialize AMQP message into EventData

Adding AmqpException for EventDataBatch

Update ReactorDispatcher to use friendly ways of fetching work.

Adding ReactorFactory for instantiating Reactor.

Adding ReactorHandler.

Add AmqpConnection, AmqpSession, AmqpLink, ConnectionState,

Add ConnectionHandler, CustomIOHandler, DispatchHandler

Add initial way to create a connection and reactor into EventHubClient

Update Sender test with correct exception and add initial EventHubClientTest

ReactorFactory: Add ability to createConnection.

Add ReactorExecutor which is responsible for looping and processing reactor work.

Add ReactorProvider that will handle creation of the Reactor instances.

Update ReactorConnection to create a ReactorExecutor and run it.

Adding ShutdownSignal and NotifierBase for state changes.

AmqpConnection and AmqpSession have State change processors.

Adding Handler (base class) and LinkHandler, SendHandler, ReceiveHandler.

Adding Track 2 SDK to pom.client.xml
@conniey
Adding testbase
22c39e8
@conniey
Fixing errors.
2074d89
@conniey
Fixing spotbug issues.
d0a78fd
@conniey conniey force-pushed the connectionCreation branch from 54f17df to d0a78fd Compare June 2, 2019 05:47
jsquire
jsquire approved these changes Jun 3, 2019
View reviewed changes
Copy link
Member

@jsquire jsquire left a comment

Choose a reason for hiding this comment

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

@conniey conniey force-pushed the connectionCreation branch from f6f9beb to 04c2a86 Compare June 3, 2019 21:07
JonathanGiles
JonathanGiles reviewed Jun 3, 2019
View reviewed changes
@conniey conniey mentioned this pull request Jun 3, 2019
Closed
2 tasks
@conniey conniey merged commit 210eaa2 into Azure:conniey/eventhubs Jun 4, 2019
@conniey conniey deleted the connectionCreation branch June 4, 2019 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonathanGiles JonathanGiles JonathanGiles left review comments

@jsquire jsquire jsquire approved these changes

@mssfang mssfang Awaiting requested review from mssfang

@ramya-rao-a ramya-rao-a Awaiting requested review from ramya-rao-a

@ramya0820 ramya0820 Awaiting requested review from ramya0820

@YijunXieMS YijunXieMS Awaiting requested review from YijunXieMS

Assignees

@conniey conniey

Labels
Client This issue points to a problem in the data-plane of the library. Event Hubs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@conniey @azuresdkci @jsquire @JonathanGiles