-
Notifications
You must be signed in to change notification settings - Fork 493
Refactor of Microsoft.Bot.Builder.Azure.Queues. #4692
Changes from all commits
cd143b8
5bad8d6
7e9f010
b349eeb
47a640e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,23 +30,6 @@ | |
| "=addHours(utcNow(), 1)" | ||
| ] | ||
| }, | ||
| "connectionString": { | ||
| "$ref": "schema:#/definitions/stringExpression", | ||
| "title": "Connection String", | ||
| "description": "Connection string for connecting to an Azure Storage account.", | ||
| "examples": [ | ||
| "=settings.connectionString" | ||
| ] | ||
| }, | ||
| "queueName": { | ||
| "$ref": "schema:#/definitions/stringExpression", | ||
| "title": "Queue Name", | ||
| "description": "Name of the queue that your azure function is bound to.", | ||
| "examples": [ | ||
| "activities" | ||
| ], | ||
| "default": "activities" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like the class was changed to match the schema name is good, but if we have shipped this it is a breaking change. Have we shipped it? #Resolved
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shipped as a preview, so we should be safe to change it. In reply to: 501232641 [](ancestors = 501232641) |
||
| }, | ||
| "value": { | ||
| "$ref": "schema:#/definitions/valueExpression", | ||
| "title": "Value", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.Bot.Schema; | ||
|
|
||
| namespace Microsoft.Bot.Builder | ||
| { | ||
| /// <summary> | ||
| /// A base class for enqueueing an Activity for later processing. | ||
| /// </summary> | ||
| public abstract class QueueStorage | ||
| { | ||
| /// <summary> | ||
| /// Enqueues an Activity for later processing. The visibility timeout specifies how long the message should be invisible | ||
| /// to Dequeue and Peek operations. The message content must be a UTF-8 encoded string that is up to 64KB in size. | ||
| /// </summary> | ||
| /// <param name="activity">The <see cref="Activity"/> to be queued for later processing.</param> | ||
| /// <param name="visibilityTimeout"> Visibility timeout. Optional with a default value of 0. Cannot be larger than 7 days. </param> | ||
| /// <param name="timeToLive">Specifies the time-to-live interval for the message.</param> | ||
| /// <param name="cancellationToken">Cancellation token for the async operation.</param> | ||
| /// <returns>A result string.</returns> | ||
| public abstract Task<string> QueueActivityAsync(Activity activity, TimeSpan? visibilityTimeout = null, TimeSpan? timeToLive = null, CancellationToken cancellationToken = default); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| using Microsoft.Bot.Builder.Adapters; | ||
| using Microsoft.Bot.Builder.Azure.Queues; | ||
| using Microsoft.Bot.Builder.Dialogs; | ||
| using Microsoft.Bot.Builder.Dialogs.Adaptive.Actions; | ||
| using Microsoft.Bot.Builder.Tests; | ||
| using Microsoft.Bot.Schema; | ||
| using Newtonsoft.Json; | ||
|
|
@@ -40,13 +41,15 @@ public async Task ContinueConversationLaterTests() | |
| .UseStorage(new MemoryStorage()) | ||
| .UseBotState(new ConversationState(new MemoryStorage()), new UserState(new MemoryStorage())); | ||
|
|
||
| var queueStorage = new AzureQueueStorage(ConnectionString, queueName); | ||
| var dm = new DialogManager(new ContinueConversationLater() | ||
| { | ||
| ConnectionString = ConnectionString, | ||
| QueueName = queueName, | ||
| Date = "=addSeconds(utcNow(), 2)", | ||
| Value = "foo" | ||
| }); | ||
|
|
||
| dm.InitialTurnState.Set<QueueStorage>(queueStorage); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there any samples which did this in the old way? Do we need a sample? #Resolved
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are no samples to update with this change. We do have a plan to add this to a sample later. In reply to: 501233216 [](ancestors = 501233216) |
||
|
|
||
| await new TestFlow((TestAdapter)adapter, dm.OnTurnAsync) | ||
| .Send("hi") | ||
| .StartTestAsync(); | ||
|
|
||
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.
Not sure if we still do this, but I think we need to update test.schema with the new name (I don't know if test.schema gets regenerated on the build server but the copy in source control has the old name)
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.
Do you mean the schema in libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Actions? If so, that change is included in this PR. I have not found any other references in the source. Please let me know.
In reply to: 501018849 [](ancestors = 501018849)
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.
I searched on my local copy of schema.tests and I see a reference to the old name, I do see it too on your branch here, here and in a couple of other places.
It seems that tests.schema gets regenerated by the unit tests so I am not sure if this is relevant or not (if tests.schema is regenerated every time we may want to added to the gitignore, would this be the case @chrimc62?)