Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

[SDK][Bot-Azure] Add AzureQueueStorage component #1033

Merged

Conversation

VictorGrycuk
Copy link
Contributor

Fixes #909

Description

Implements AzureQueueStorage class and its unit test. This PR also fixes a few disparities with C# present in different classes.

Specific Changes

  • Add AzureQueueStorage class to Bot Azure library
  • Add unit test for AzureQueueStorage
  • Add ActivityEventNames class to Bot Schema library
  • Add missing locale assignation in TestAdapter
  • Fix unnecessary lowercasing in TestAdapter
  • Fix missing assignation of service URL in ConversationReference's getContinuationActivity() method
  • Fix the test ConfirmPrompt_Locale_Override_ChoiceDefaults of ConfirmPromptLocTests, by adding a missing assignation of the custom locale

Testing

These implementations were successfully validated by running the build and unit tests

image

image

@VictorGrycuk
Copy link
Contributor Author

Hi @tracyboehrer, the unit test for this implementation requires Azure Storage Emulator, just like the CosmosDbPartitionedStorage.

We found this prep-test in BotBuilder JS, similar to CosmosDbPartitionedStorage.

try {
queueClient.create();
} catch (QueueStorageException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it acceptable for the code to continue at this point or should we return here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @LeeParrishMSFT, we will review this and apply the corresponding fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @LeeParrishMSFT, we improved this behaviour to throw a RuntimeException like the CosmosDbPartitionedStorage does.

@tracyboehrer
Copy link
Member

@Batta32 Can you change this, and the other Storage impl to follow the pattern in CosmosDbPartitionStorageTests. I changed 'assertEmulator' to 'runIfEmulator'.

The relevant code is:

    private boolean runIfEmulator() {
        if (!emulatorIsRunning) {
            System.out.println(NO_EMULATOR_MESSAGE);
            return false;
        }

        return true;
    }

Then (for example):

    @Test
    public void createObjectCosmosDBPartitionTest() {
        if (runIfEmulator()) {
            super.createObjectTest(storage);
        }
    }

@Batta32
Copy link
Contributor

Batta32 commented Mar 22, 2021

Sure @tracyboehrer, we will apply those changes to follow the pattern of CosmosDbPartitionStorageTests in this PR and in PR #1066.

VictorGrycuk and others added 2 commits March 22, 2021 11:33
* Apply Tracy's feedback

* Update libraries/bot-azure/src/test/java/com/microsoft/bot/azure/AzureQueueTests.java

Co-authored-by: Martin Battaglino <[email protected]>

Co-authored-by: Martin Battaglino <[email protected]>
@Batta32
Copy link
Contributor

Batta32 commented Mar 22, 2021

@tracyboehrer, we finished applying the requested change here and also, we merged main into this branch.

We will continue working on the PR #1066.

@VictorGrycuk
Copy link
Contributor Author

@tracyboehrer we were able to reproduce the error 400 by downgrading the azure-storage-queue to a previous version (12.0.0). We also tested from version 12.1.0 to 12.8.0, and the error is not present when using the Azure Storage Emulator v5.10.0.

The issue seems to be a mismatch between the version of the emulator and of the library.
Could you please validate the version of the Azure Storage Emulator running in the pipelines?

@tracyboehrer
Copy link
Member

@VictorGrycuk OK. I think we have this sorted out. At least the PR build succeeded. If you can update from main again I promise we'll get this merged.

@Batta32
Copy link
Contributor

Batta32 commented Mar 23, 2021

@tracyboehrer done! The build passed 🎉!

@tracyboehrer tracyboehrer merged commit b7c65ca into microsoft:main Mar 23, 2021
@Batta32 Batta32 deleted the external/southworks/AzureQueue/base branch March 23, 2021 19:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bot-azure, AzureQueueStorage
4 participants