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.

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

Enable DTFx.AzureStorage v1.x backward compatibility support #1120

Merged
merged 5 commits into from
Jun 25, 2024

Conversation

nytian
Copy link
Collaborator

@nytian nytian commented Jun 24, 2024

DTFx.AzureStorage v1.x and v2.x use different Azure Storage SDK versions, each with its own encoding method. This PR adds backward compatibility to DTFx.AzureStorage v1.x, ensuring that queue messages created with DTFx.AS v2 remain readable after downgrading to DTFx.AS v1.

The RawProperty is an internal attribute of CloudQueueMessage, and we use System.Reflection to access it. The included catch block should only be triggered in the scenario described above.

End-to-End tests have been done.

@nytian nytian requested a review from davidmrdavid June 24, 2024 20:51
Copy link
Collaborator

@davidmrdavid davidmrdavid left a comment

Choose a reason for hiding this comment

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

Some minor feedback.
@nytian - Given this created an outage, I think we should have an automated test for this, either a unit test, or an E2E test. I think an E2E test, written as a github action, might be simplest. We just need to populate a DTFx. v1 queue with a DTFx v2 message, and see that it is able to be processed by DTFx.AS v1

src/DurableTask.AzureStorage/Storage/QueueMessage.cs Outdated Show resolved Hide resolved
@nytian
Copy link
Collaborator Author

nytian commented Jun 24, 2024

Some minor feedback. @nytian - Given this created an outage, I think we should have an automated test for this, either a unit test, or an E2E test. I think an E2E test, written as a github action, might be simplest. We just need to populate a DTFx. v1 queue with a DTFx v2 message, and see that it is able to be processed by DTFx.AS v1

@davidmrdavid Thanks for the review! I wonder what do you mean by a E2E test within GitHub actions? You want me to add a new folder/file that include the e2e test and add it to our workflow?

@davidmrdavid
Copy link
Collaborator

@nytian - actually, let me take that back - I just realized that writing this E2E test will be difficult for DTFx.AS V1 since DTFx.AS V1 doesn't work well with Azurite.

For now - don't worry about this. I trust your manual test, let's just make a note to perform another one right before release.

@nytian nytian merged commit d75526f into main Jun 25, 2024
3 checks passed
@nytian nytian deleted the nytian/azure-storage-backward branch June 25, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants