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
Update Azure Service Bus components to track2 SDK #1702
Conversation
cadd5ab
to
bbb776e
Compare
@@ -489,7 +487,6 @@ func (a *azureServiceBus) Subscribe(req pubsub.SubscribeRequest, handler pubsub. | |||
attempts := readAttemptsStale() | |||
if attempts == 0 { | |||
a.logger.Errorf("Subscription to topic %s lost connection, unable to recover after %d attempts", sub.topic, a.metadata.MaxReconnectionAttempts) |
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.
This is unchanged, but wondering if we should make the app crash here (Fatalf
)? Otherwise, the error would get logged and the app continues to work but without a connection to Service Bus.
} | ||
} | ||
|
||
func TestNewPubsubMessageFromAzServiceBusMessage(t *testing.T) { |
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.
This test can't be implemented with track2 SDKs. The method to create these objects is un-exported.
Can be re-added if Azure/azure-sdk-for-go#17848 is merged
fba3884
to
8d97718
Compare
Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
Includes some minor refactoring of auth code Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: halspang <halspang@microsoft.com> Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
) * certification test for eventhubs binding Signed-off-by: tanvigour <tanvi.gour@gmail.com> * modified go.mod and go.sum Signed-off-by: tanvigour <tanvi.gour@gmail.com> * Add connection string testing Signed-off-by: tanvigour <tanvi.gour@gmail.com> * iothub testing Signed-off-by: tanvigour <tanvi.gour@gmail.com> * address feedback and run test Signed-off-by: tanvigour <tanvi.gour@gmail.com> * Install Azure CLI IOT hub extension Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com> * make modtidy-all Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com> * covering all eventhubs test cases Signed-off-by: tanvigour <tanvi.gour@gmail.com> * dependency changes after go modtidy-all Signed-off-by: tanvigour <tanvi.gour@gmail.com> Co-authored-by: Bernd Verst <4535280+berndverst@users.noreply.github.com> Co-authored-by: Yaron Schneider <schneider.yaron@live.com> Co-authored-by: Looong Dai <long.dai@intel.com> Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: pigletfly <wangbing.adam@gmail.com> Co-authored-by: Yaron Schneider <schneider.yaron@live.com> Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
* Updated to Go 1.18 Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> * Added go.work file With Go 1.18, this allows gopls (the Go language server used for example in VS Code) to work inside test apps too. See: https://go.dev/doc/tutorial/workspaces Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> * Removed go.work Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> * 💄 Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> Co-authored-by: Bernd Verst <4535280+berndverst@users.noreply.github.com> Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
* Add metadata property to configure BatchingMaxSize&batchingMaxMessages in Pulsar Signed-off-by: saberwang <saberwang@hotmail.com> * sort field Signed-off-by: saberwang <saberwang@hotmail.com> * [pubsub]fix unit test bug Signed-off-by: saberwang <saberwang@hotmail.com> * remove unrelated changes Signed-off-by: saberwang <saberwang@hotmail.com> * Delete hard coded Metadata Signed-off-by: saberwang <saberwang@hotmail.com> * remove .history Signed-off-by: saberwang <saberwang@hotmail.com> * restore .gitignore Signed-off-by: saberwang <saberwang@hotmail.com> * Hard coding default values and adding 'BatchingMaxPublishDelay' metadata Signed-off-by: saberwang <saberwang@hotmail.com> * fix code format Signed-off-by: saberwang <saberwang@hotmail.com> * formatting code Signed-off-by: saberwang <saberwang@hotmail.com> Co-authored-by: Looong Dai <long.dai@intel.com> Co-authored-by: Bernd Verst <4535280+berndverst@users.noreply.github.com> Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
The methods to create a message with a body are not exported Signed-off-by: Alessandro Segala (ItalyPaleAle) <43508+ItalyPaleAle@users.noreply.github.com>
8d97718
to
957bb8d
Compare
…nto service-bus-track2
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
/ok-to-test |
@ItalyPaleAle please fix linter issues. Also use |
This greatly simplifies certain parts of the code, reducing the number of goroutines and likely improving performance. Performance for end-users improves too as there's no need anymore to pause for 2 seconds every time that we reach `maxActiveMessages`. Additionally, with this change the config options `prefetchCount` and `maxActiveMessagesRecoveryInSec` are removed as unnecessary anymore. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
At the moment this breaks EventHubs Binding, PubSub and Service Bus Queues Binding. Please take a look by running the certification and conformance tests via |
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
/ok-to-test |
@berndverst This is ready for review IMHO.
Once this is merged, I'll need to update the docs because of metadata changes, as well as our E2E tests if needed. |
Eventhubs Binding conformance test is failing (you can see in the results of your /ok-to-test run) |
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
/ok-to-test |
I'll look into the Service Bus binding failure tomorrow. For Event Hubs, that's very odd because I did not touch that module besides for a minimal change that was required. However, it looked like the SDK got upgraded (which was un-intentional). I've reverted the change to the SDK and let's see if that fixes the tests. |
This reverts commit 212220a. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
98f0043
to
3bf733d
Compare
…ng cert test Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
191f725
to
86d5798
Compare
In case of a failure in the handler (ie. users' code), the message should be correctly re-enqueued, which means that messages will be re-delivered later and won't be in order. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
@berndverst I have fixed all tests that were failing because of changes in my code. After fixing dependencies, there were 2 tests that fail.
(On why the Event Hubs test is failing: the test Test runs: |
Codecov Report
@@ Coverage Diff @@
## master #1702 +/- ##
==========================================
- Coverage 36.37% 36.24% -0.13%
==========================================
Files 166 173 +7
Lines 15488 15557 +69
==========================================
+ Hits 5633 5639 +6
- Misses 9228 9287 +59
- Partials 627 631 +4
Continue to review full report at Codecov.
|
Description
Move the Azure Service Bus components (binding and pubsub) to the new "track2" SDKs. Thanks a lot to @halspang for the initial work in #1648!
Thanks to the update and some refactoring work, two metadata keys for Azure Service Bus Pub/Sub are not needed anymore:
prefetchCount
maxActiveMessagesRecoveryInSec
Notes:
This requires Go 1.18 so it depends on Updated to Go 1.18 #1697 (which depends on Use revive instead of golint #1685 itself)Issue reference
Please reference the issue this PR will:
closes #1532
closes #1531
closes #1701