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

feat: allow create SAS token for azure storage #306

Merged
merged 5 commits into from
May 8, 2023

Conversation

godruoyi
Copy link
Contributor

@godruoyi godruoyi commented May 5, 2023

close #301

This PR is support azure SAS token(account SAS), we will try to generate SAS token when call sign_query without setting SharedAccessSignature(token) in Credential.

Cargo.toml Outdated Show resolved Hide resolved
src/azure/storage/signer.rs Outdated Show resolved Hide resolved
@godruoyi godruoyi requested a review from Xuanwo May 5, 2023 09:07
src/time.rs Outdated Show resolved Hide resolved
tests/azure/storage.rs Show resolved Hide resolved
@godruoyi godruoyi requested a review from Xuanwo May 5, 2023 12:12
Copy link
Owner

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Mostly LGTM!

src/azure/storage/signer.rs Outdated Show resolved Hide resolved
Copy link
Owner

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Mostly LGTM!

Copy link
Owner

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

Comment on lines +139 to +145
pub fn sign_query(
&self,
req: &mut impl SignableRequest,
expire: Duration,
cred: &Credential,
) -> Result<()> {
let ctx = self.build(req, SigningMethod::Query(expire), cred)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Xuanwo Please note that we have modified the signature of this function, and we need to pay attention to compatibility when releasing the version.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks! We should release v0.10 instead!

@Xuanwo Xuanwo merged commit e227a2d into Xuanwo:main May 8, 2023
@godruoyi godruoyi deleted the allow_create_sas_token branch May 8, 2023 04:01
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.

azure storage: Allow create SAS token
2 participants