Service client samples, bug fix #3261
Conversation
timtay-microsoft
commented
Apr 10, 2023
timtay-microsoft
commented
- Add messaging client sample
- Add retry logic to file upload notification sample
- Fix bug where error processor would execute 3 times on a connection loss event
- The same event handler was hooked up to the connection drop, session drop, and link drop event. Now the link drop event just triggers the session to close and the session drop event just triggers the connection to close. Only the connection layer can trigger the "Connection lost" event
…csharp into timtay/samples2
timtay-microsoft
requested review from
drwill-ms,
abhipsaMisra,
andyk-ms,
brycewang-microsoft,
tmahmood-microsoft and
patilsnr
as code owners
| @@ -69,41 +72,8 @@ private async Task ReceiveFileUploadNotificationsAsync(string targetDeviceId, Ca | |||
|
|
|||
| _logger.LogInformation($"Listening for file upload notifications from the service."); | |||
|
|
|||
| Task<AcknowledgementType> FileUploadNotificationCallback(FileUploadNotification fileUploadNotification) | |||
There was a problem hiding this comment.
Moving this to a private function outside of this function so this sample is more readable
| @@ -20,7 +20,6 @@ internal class AmqpCbsSessionHandler : IDisposable | |||
| private readonly IotHubConnectionProperties _credential; | |||
|
|
|||
| private AmqpCbsLink _cbsLink; | |||
| private readonly EventHandler _connectionLossHandler; | |||
There was a problem hiding this comment.
Unused. CBS links don't allow for subscribing to link drop events for some reason.
...getting started/FileUploadNotificationReceiverSample/FileUploadNotificationReceiverSample.cs
Outdated
Show resolved
Hide resolved
iothub/service/samples/how to guides/MessagingClientSample/MessagingClientSample.cs
Outdated
Show resolved
Hide resolved
| <PackageReference Include="CommandLineParser" Version="2.9.1" /> | ||
| </ItemGroup> | ||
|
|
||
| <!--<ItemGroup Condition="'$(Configuration)' == 'Release'"> |
There was a problem hiding this comment.
I see this same block in all the other samples, so I believe David has something in mind with this commented out code.
There was a problem hiding this comment.
We can put it in after we GA, so customers will get the shipped nuget tested if they run Release.
We can choose to only use project reference tho. If we add new functionality to an SDK and use it in a sample, it'd otherwise be broken in Release until after we release, so this may not have been my brightest idea.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>net6.0</TargetFramework> | ||
| <LangVersion>9.0</LangVersion> |
| @@ -35,25 +36,25 @@ protected internal AmqpSessionHandler() | |||
| /// upload notifications or receiving feedback messages. | |||
| /// </summary> | |||
| /// <param name="linkAddress">The link address to receive from/send to.</param> | |||
| /// <param name="connectionLossHandler">The handler to invoke if this session or its links are dropped.</param> | |||
| /// <param name="sessionLossHandler">The handler to invoke if this session or its links are dropped.</param> | |||
There was a problem hiding this comment.
Should the comment still mention dropped links?
There was a problem hiding this comment.
Yes because this handler is still eventually invoked if its links are dropped. A link drop triggers the session to close.
Follow-up on feedback on #3261
Follow-up on feedback on #3261
