Skip to content

Comments

refactor(services/yandex_disk): Move raw request to core#6090

Merged
Xuanwo merged 9 commits intoapache:mainfrom
jorgehermo9:yandex_disk-request-refactor
Apr 27, 2025
Merged

refactor(services/yandex_disk): Move raw request to core#6090
Xuanwo merged 9 commits intoapache:mainfrom
jorgehermo9:yandex_disk-request-refactor

Conversation

@jorgehermo9
Copy link
Contributor

}

async fn read(&self, path: &str, args: OpRead) -> Result<(RpRead, Self::Reader)> {
// TODO: move this out of reader.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it okay to remove this TODO? Is this addressed now? I see the get_download_url and download(download_url) pattern in few backend.rs and it think it is the recommended approach, right? Or should we move both get_download_url and download(download_url) into a new read function inside core.rs so the backend is abstracted from this two-step flow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see that b2 service encapsulates both download_url and download in core

let resp = self.get_upload_url().await?;

Should we do the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per #6089 (comment), I will refactor this also

@jorgehermo9 jorgehermo9 marked this pull request as ready for review April 26, 2025 12:18
@jorgehermo9 jorgehermo9 requested a review from Xuanwo as a code owner April 26, 2025 12:18
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" labels Apr 26, 2025
Copy link
Member

@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.

Thank you for this change.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 27, 2025
@Xuanwo Xuanwo merged commit d7e506d into apache:main Apr 27, 2025
86 of 87 checks passed
@jorgehermo9 jorgehermo9 deleted the yandex_disk-request-refactor branch April 27, 2025 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants