Skip to content

feat(services/gdrive): Add read & write & delete support for GoogleDrive#2184

Merged
Xuanwo merged 1 commit intoapache:mainfrom
Young-Flash:gdrive
May 1, 2023
Merged

feat(services/gdrive): Add read & write & delete support for GoogleDrive#2184
Xuanwo merged 1 commit intoapache:mainfrom
Young-Flash:gdrive

Conversation

@Young-Flash
Copy link
Copy Markdown
Member

@Young-Flash
Copy link
Copy Markdown
Member Author

What do you think is the appropriate place to store the cache of path to id mapping? @Xuanwo

@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented May 1, 2023

What do you think is the appropriate place to store the cache of path to id mapping? @Xuanwo

A hashmap in core is good enough.

Comment thread core/src/types/scheme.rs Outdated
@Young-Flash
Copy link
Copy Markdown
Member Author

A hashmap in core is good enough.

I mean who own the hashmap. An intuitive idea is to make it a member of GoogleDriveBackend, but modifying this hashmap in get_file_id_by_path() requires &mut self, while there is &self in gdrive_get(), read()

@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented May 1, 2023

I mean who own the hashmap.

An Arc<Mutex<HashMap>> in GoogleDriveCore, and GoogleDriveBackend will hold a core: Arc<GoogleDriveCore>

Comment thread core/src/services/gdrive/writer.rs Outdated
@Young-Flash
Copy link
Copy Markdown
Member Author

I'd like to left HashMap and other op in next PR to avoid this PR too large. Is there any other problem?

Comment thread core/src/services/gdrive/backend.rs Outdated
Comment thread core/src/services/gdrive/backend.rs
Comment thread core/src/services/gdrive/backend.rs Outdated
Comment thread core/src/services/gdrive/backend.rs Outdated
Comment thread core/src/services/gdrive/backend.rs Outdated
Comment thread core/src/services/gdrive/backend.rs Outdated
Comment thread core/src/services/gdrive/backend.rs
Comment thread core/src/services/gdrive/backend.rs Outdated
Comment thread core/src/types/scheme.rs Outdated
@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented May 1, 2023

BTW, this branch seems contained not related commits.

@Young-Flash
Copy link
Copy Markdown
Member Author

BTW, this branch seems contained not related commits.

It's wired, it occurs after I resolved the conflicts.

@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented May 1, 2023

It's wired, it occurs after I resolved the conflicts.

You can try git reset origin/main and do a force push.

Copy link
Copy Markdown
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.

Thanks a lot!

@Xuanwo Xuanwo merged commit bf26408 into apache:main May 1, 2023
@Young-Flash Young-Flash deleted the gdrive branch May 1, 2023 14:19
@Young-Flash Young-Flash restored the gdrive branch May 1, 2023 14:19
@Xuanwo Xuanwo mentioned this pull request May 6, 2023
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.

4 participants