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

Add Blob Copy & Page Blob support to by SQL based metadata implementation in Blob API and Sync Loki and SQL metadata stores #2225

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

mahmoudbahaa
Copy link

Add Blob Copy & Page Blob support to by SQL based metadata implementation in Blob API. This makes the SQL based metadata implementation in sync with File based one. (issue #2224)

I divided the PR into very small mini commits for better visibility but most of these commits are small either in scope or code.

Changes are as follows:

In code: ([SqlBlobMetadataStore.ts])

  1. Changed properties from separate fields into a single one to allow serializing de-serializing of remaining properties as well.
  2. Added missing await from some async functions in SqlBlobMetadataStore
  3. Ported copyFromURL from Loki to SQL blobMeatadataStore
  4. Ported appendBlock from Loki to SQL blobMeatadataStore
  5. Ported clearRange from Loki to SQL blobMeatadataStore
  6. Ported getPageRanges from Loki to SQL blobMeatadataStore
  7. Ported resizePageBlob from Loki to SQL blobMeatadataStore
  8. Ported updateSequenceNumber from Loki to SQL blobMeatadataStore
  9. Return blobCommittedBlockCount for append blob in getBlobProperties in SQL blobMetadataStore
  10. Check for CopyIfExists condition in startCopyFromURL in SQL blobMetadaStore

In Tests:

  1. Add @Sql to all @loki tests
  2. Ignore generateAccountSASQueryParameters in sas blob tests as it depend on timezone and fails even then and test another component not Azurite
  3. Skip production style URL test as it fails in new sdk //To be fixed
  4. Remove skipping of "list uncimmited blob from container" in blob api container test since now it is working
  5. Added await to 2 async functions in 2 tests that was causing these 2 tests to be flaky

Others:

  1. Changed readme and changelog to relfect sync btween Loki and SQL blob metadata stores.

Note: I recommend removing the @loki and @Sql all together and run all test cases without grep for both loki and sql to avoid descync in the future but its your call.

…rializing de-serializing of remaining properties as well.
…nd on timezone and fails even then and test another component not Azurite
@mahmoudbahaa mahmoudbahaa changed the title Add Blob Copy & Page Blob support to by SQL based metadata implementation in Blob API Add Blob Copy & Page Blob support to by SQL based metadata implementation in Blob API and Sync Loki and SQL metadata stores Oct 14, 2023
@mahmoudbahaa mahmoudbahaa reopened this Oct 14, 2023
it(`Should work with production style URL when ${productionStyleHostName} is resolvable`, async () => {
// This now doesn't work (Error: Unable to extract accountName with provided information.).
// As the sdk expect an url in the form devstoreaccount1.blob.localhost instead of devstoreaccount1.localhost
it.skip(`Should work with production style URL when ${productionStyleHostName} is resolvable`, async () => {
Copy link
Member

Choose a reason for hiding this comment

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

This looks already won't run as don't have loki or sql tag, so looks don't need skip it.

@@ -488,8 +488,6 @@ This feature is in preview, when Azurite changes database table schema, you need

> Note. Need to manually create database before starting Azurite instance.

> Note. Blob Copy & Page Blob are not supported by SQL based metadata implementation.

Copy link
Member

Choose a reason for hiding this comment

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

The support matrix section in ReadMe.md should also be updated.

Azurite/README.md

Line 1011 in 9208664

- Copy Blob From URL (Only supports copy within same Azurite instance, only on Loki)

contentProperties: {
type: "VARCHAR(1023)"
properties: {
type: "VARCHAR(4095)"
Copy link
Member

Choose a reason for hiding this comment

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

Will this cause the saved blobs in old Azurite version, can't be read out with new Azurite version?

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.

None yet

2 participants