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

[storage-file-share] Migrate to core-rest-pipeline #26102

Merged
merged 12 commits into from
Jun 7, 2023

Conversation

xirzec
Copy link
Member

@xirzec xirzec commented Jun 5, 2023

Packages impacted by this PR

@azure/storage-file-share

Issues associated with this PR

#15813

Describe the problem that is addressed by this PR

This PR migrates storage-file-share to the new core pipeline in the same way that storage-blob and storage-file-datalake were migrated. There are no changes to the public surface and existing recorded tests still pass.

Provide a list of related PRs (if any)

#24141, #24835

@xirzec xirzec added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Jun 5, 2023
@xirzec xirzec self-assigned this Jun 5, 2023
@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-storage-file-share

"dist/",
"dist-esm/src/",
"dist-esm/storage-blob/src/",
Copy link
Member

Choose a reason for hiding this comment

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

do we need to pack all of storage-blob, or do we plan to move common ones to storage-common?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, ultimately I think this is solved by doing a storage-common package, but I wasn't able to just move those source files there since without it being a package they can't import anything from node_modules. I could probably carefully craft packing less, but if we change imports things might break so it felt safest to do overpack until we can centralize.

Open to other ideas too!

Copy link
Member

Choose a reason for hiding this comment

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

The current storage way is to keep duplicate of same files in each packages, which has maintainence problems too. Yes hopefully we can move to the storage-common package soon.

@xirzec
Copy link
Member Author

xirzec commented Jun 6, 2023

/azp run js - storage-file-share- tests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@xirzec
Copy link
Member Author

xirzec commented Jun 6, 2023

/azp run js - storage-file-share - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xirzec
Copy link
Member Author

xirzec commented Jun 6, 2023

/azp run js - storage-file-share - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xirzec
Copy link
Member Author

xirzec commented Jun 7, 2023

Merging this to keep up the momentum, but I'm happy to address any remarks/concerns you have @EmmaZhu in a follow-up PR. The meta-plan is to finish getting all Storage SDKs updated in main and then do a round of reviews/testing.

@xirzec xirzec merged commit e7f9ea3 into Azure:main Jun 7, 2023
@xirzec xirzec deleted the storage-file-share-v2 branch June 7, 2023 18:30
minhanh-phan pushed a commit to minhanh-phan/azure-sdk-for-js that referenced this pull request Jun 12, 2023
### Packages impacted by this PR

`@azure/storage-file-share`

### Issues associated with this PR

Azure#15813

### Describe the problem that is addressed by this PR

This PR migrates storage-file-share to the new core pipeline in the same
way that storage-blob and storage-file-datalake were migrated. There are
no changes to the public surface and existing recorded tests still pass.

### Provide a list of related PRs _(if any)_

Azure#24141, Azure#24835
xirzec added a commit that referenced this pull request Jun 15, 2023
### Packages impacted by this PR

`@azure/storage-queue`

### Issues associated with this PR

Fixes #15813
Closes #15594

### Describe the problem that is addressed by this PR

This PR migrates storage-queue to the new core pipeline in the same way
that storage-file-share was migrated. There are no changes to the public
surface and existing recorded tests still pass.

### Provide a list of related PRs _(if any)_

#26102
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants