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.

Sign up for GitHub

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

Jump to bottom

CRC: Always Structured Message #44955

Merged

jaschrep-msft merged 17 commits into Azure:feature/storage/ContentValidation from jaschrep-msft:sm-always-on-crc

Jul 16, 2024

Member

jaschrep-msft commented Jul 11, 2024

Due to limitations of the files service, changing CRC behavior to always use structured message.

Affects blobs and datalake. Shares will be updated to match later.
Contains temporary disabling of master CRC in some cases for simpler PR. TODOs in place.
Doesn't expose the final transactional CRC to the caller just yet. TODOs in place.

jaschrep-msft added 9 commits

July 8, 2024 10:15


          blockblob working


          revert testing change

bc6f765


          page/append

f11ea41


          datalake file

476b00a


          testfix

d31eea1


          bug fixes | test fixes

14814cd


          disable new API for presenting CRC from structured message

a3f6a5c


          fix nunit

0b992f9


          whitespace

3c75c62

jaschrep-msft requested a review from jalauzon-msft

July 11, 2024 15:56

jaschrep-msft requested review from seanmcc-msft, amnguye, schaabs and tg-msft as code owners

July 11, 2024 15:56

github-actions bot added the Storage label

jalauzon-msft approved these changes

View reviewed changes

sdk/storage/Azure.Storage.Common/src/Shared/StructuredMessagePrecalculatedCrcWrapperStream.cs


		namespace Azure.Storage.Shared;

		internal class StructuredMessagePrecalculatedCrcWrapperStream : Stream

Member

jalauzon-msft Jul 11, 2024

Any way to share some logic between this and the regular encode stream? If it's not fairly easy, it may not be worth it but thought I'd ask.

I didn't look at the implementation too closely because I assumed it was similar and mainly only different with the checksum.

Member Author

jaschrep-msft Jul 12, 2024

I tried and it wasn't great to get working. Weird bugs I never managed to track down. This is a simplified version of the normal one with only one segment in the whole stream and the provided CRC pasted in.


          fix/test-proxy

948aa25

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

4 similar comments

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.


          csproj

177b1b2

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

4 similar comments

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.


          more csproj removeals

dfb53b1

This is building fine locally idk what's up

Collaborator

azure-sdk commented Jul 12, 2024

API change check

API changes are not detected in this pull request.

jaschrep-msft requested review from hallipr, weshaggard, benbp, miguhern, whisper6284, RoyHerrod and danielav7 as code owners

July 16, 2024 14:48

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

9 similar comments

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.


          Revert "Update macos image from 11 to latest (Azure#44607)"

66f51aa

this is causing too many problems. skipping macos tests for now. They'll
run when this feature branch merges into main.

This reverts commit 29e87b4.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

8 similar comments

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

Collaborator

azure-sdk commented Jul 16, 2024

API change check

API changes are not detected in this pull request.

jaschrep-msft merged commit 6231eb5 into Azure:feature/storage/ContentValidation

11 of 15 checks passed

jaschrep-msft added a commit to jaschrep-msft/azure-sdk-for-net that referenced this pull request


          CRC: Always Structured Message (Azure#44955)

* blockblob working

* revert testing change

* page/append

* datalake file

* testfix

* bug fixes | test fixes

* disable new API for presenting CRC from structured message

* fix nunit

* whitespace

* fix/test-proxy

* csproj

* more csproj removeals

This is building fine locally idk what's up

* Trigger Fresh Build

* fileshare testproxy

* fix mock

* Update macos image from 11 to latest (Azure#44607)

* Update macos image from 11 to latest

* Update eng/pipelines/templates/jobs/ci.mgmt.yml

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

---------

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

* Revert "Update macos image from 11 to latest (Azure#44607)"

this is causing too many problems. skipping macos tests for now. They'll
run when this feature branch merges into main.

This reverts commit 29e87b4.

---------

Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

jaschrep-msft added a commit that referenced this pull request


          Structured message cherrypick stg96 (#45496)

8e97158

* Structured Message Decode Stream (#42079)

* Initial implementation and basic test

* seek/write tests

* fix test param

* fix exceptions

* Content validation update spec (#42191)

* enum rename and footer read/write methods

* align encode/decode tests | update encoding stream

* decode stream footer

* rename

* decode tests & bugfixes (#42256)

* decode tests & bugfixes

* roundtrip tests

* more tests

* better errors | remove duplicate test

* test coverage | exception message (#42363)

* Structured Message Decode: Validate Content Length (#42370)

* validate stream length

* tests

* stageblock | appendblock | putpages StructuredMessage (#42699)

* regenerate and stage block uses structured message

* page and append

* testproxy

* cleanup

* fix datalake/share tests

* testproxy

* testproxy

* re-add null-safe access

* GET Blob Structured Message (#42959)

* download range structured message

* testproxy

* throw when service fails to give back structured message

* test

* testproxy

* PUT Blob Structured Message (#43130)

* putblob structured message

* testproxy

* fixes

* Structured Message: DataLake Append (#43275)

* datalake append

* null fix

* fixes

* Retriable decode (#44155)

* retriable decode

* rewind mock test

* bugfix

* bugfix

* tests

* Download retriable stream structured message (#44176)

* blobs retriable structured message download

* test proxy

* testproxy

* remove commented code

* CRC: Always Structured Message (#44955)

* blockblob working

* revert testing change

* page/append

* datalake file

* testfix

* bug fixes | test fixes

* disable new API for presenting CRC from structured message

* fix nunit

* whitespace

* fix/test-proxy

* csproj

* more csproj removeals

This is building fine locally idk what's up

* Trigger Fresh Build

* fileshare testproxy

* fix mock

* Update macos image from 11 to latest (#44607)

* Update macos image from 11 to latest

* Update eng/pipelines/templates/jobs/ci.mgmt.yml

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

---------

Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

* Revert "Update macos image from 11 to latest (#44607)"

this is causing too many problems. skipping macos tests for now. They'll
run when this feature branch merges into main.

This reverts commit 29e87b4.

---------

Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

* Enable fileshare crc (#45124)

* impl

* testproxy

* shares fix

* testproxy

* block blob fix (#45129)

* block blob fix

* testproxy

* rename and validation (#45160)

* rename and validation

* fix

* crc tracking converted to longs (#45307)

* Crc reporting pt2 (#45447)

* expose crc from structured message

* testproxy

* undo typo

* exportapi

* testproxy

* remove unused parameter

* add `ExpectTrailingDetails` to download response

* fix test inconsistency

* fix auto

---------

Co-authored-by: Wes Haggard <weshaggard@users.noreply.github.com>
Co-authored-by: Ben Broderick Phillips <ben@benbp.net>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jalauzon-msft jalauzon-msft approved these changes

seanmcc-msft Awaiting requested review from seanmcc-msft seanmcc-msft is a code owner

amnguye Awaiting requested review from amnguye amnguye is a code owner

schaabs Awaiting requested review from schaabs schaabs is a code owner

tg-msft Awaiting requested review from tg-msft tg-msft is a code owner

hallipr Awaiting requested review from hallipr

weshaggard Awaiting requested review from weshaggard

benbp Awaiting requested review from benbp

miguhern Awaiting requested review from miguhern

whisper6284 Awaiting requested review from whisper6284

RoyHerrod Awaiting requested review from RoyHerrod

danielav7 Awaiting requested review from danielav7

Labels

Storage