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.

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

feat(service/gdrive): add gdrive list support #3025

Merged
merged 3 commits into from
Sep 11, 2023
Merged

feat(service/gdrive): add gdrive list support #3025

merged 3 commits into from
Sep 11, 2023

Conversation

Young-Flash
Copy link
Member

No description provided.

@Young-Flash Young-Flash requested a review from Xuanwo as a code owner September 8, 2023 12:50
Xuanwo
Xuanwo reviewed Sep 8, 2023
View reviewed changes
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.

Great! I left some comments.

By the way, since GitHub cannot perform integration tests for gdrive, could you please run those tests locally before proceeding? Thank you!

And you can create a branch directly at apache so that we can run all the tests, now that you are a committer!

core/src/services/gdrive/backend.rs Outdated Show resolved Hide resolved
core/src/services/gdrive/pager.rs Outdated Show resolved Hide resolved
@Young-Flash
Copy link
Member Author

I run cargo test gdrive --features=services-gdrive locally, got the following result, some failed test are because of google drive api User rate limit exceeded, maybe we can use retry layer in that test cases? And some failed because of thread '<unnamed>' panicked at 'service doesn't support list hierarchy, it must be a bug', any suggestion?

running 86 tests
test services_gdrive::test_blocking_read_range                    ... ok
test services_gdrive::test_blocking_fuzz_offset_reader            ... ok
test services_gdrive::test_blocking_read_large_range              ... ok
test services_gdrive::test_blocking_fuzz_part_reader              ... ok
test services_gdrive::test_blocking_fuzz_range_reader             ... ok
test services_gdrive::test_blocking_write_with_dir_path           ... ok
test services_gdrive::test_blocking_stat_not_exist                ... ok
test services_gdrive::test_blocking_read_not_exist                ... ok
test services_gdrive::test_check                                  ... ok
test services_gdrive::test_list_non_exist_dir                     ... FAILED
test services_gdrive::test_blocking_stat_dir                      ... ok
test services_gdrive::test_blocking_create_dir                    ... ok
test services_gdrive::test_list_dir_with_file_path                ... ok
test services_gdrive::test_list_with_start_after                  ... ok
thread '<unnamed>' panicked at 'create must succeed: PermissionDenied (permanent) at Writer::write => {
  "error": {
    "code": 403,
    "message": "User rate limit exceeded.",
    "errors": [
      {
        "message": "User rate limit exceeded.",
        "domain": "usageLimits",
        "reason": "userRateLimitExceeded"
      }
    ]
  }
}


Context:
   uri: https://www.googleapis.com/upload/drive/v3/files?uploadType=multipart
   response: Parts { status: 403, version: HTTP/2.0, headers: {"x-guploader-uploadid": "ADPycduFmdTm-n4wnDKTPIohkINp84FGAaJyB5XYskSAsrAmph40yDTFy_SS2SbBrVKQsDe9FxBYYpFBC0S3gLHroOENexDSw8RH", "cache-control": "private, max-age=0", "content-type": "application/json; charset=UTF-8", "date": "Thu, 07 Sep 2023 09:59:31 GMT", "expires": "Thu, 07 Sep 2023 09:59:31 GMT", "server": "ESF", "vary": "Origin, X-Origin", "x-content-type-options": "nosniff", "x-frame-options": "SAMEORIGIN", "x-xss-protection": "0", "content-length": "243", "alt-svc": "h3=\":443\"; ma=2592000,h3-29=\":443\"; ma=2592000"} }
   service: gdrive
   path: test_list_rich_dir/file-50
', core/tests/behavior/list.rs:191:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test services_gdrive::test_list_rich_dir                          ... FAILED
thread '<unnamed>' panicked at 'service doesn't support list hierarchy, it must be a bug', core/src/raw/oio/page/into_flat_page.rs:31:9
test services_gdrive::test_scan_root                              ... FAILED
test services_gdrive::test_blocking_delete_file                   ... ok
test services_gdrive::test_blocking_stat_file                     ... ok
test services_gdrive::test_blocking_stat_with_special_chars       ... ok
test services_gdrive::test_blocking_write_file                    ... ok
test services_gdrive::test_rename_non_existing_source             ... ok
test services_gdrive::test_list_empty_dir                         ... ok
test services_gdrive::test_blocking_write_with_special_chars      ... ok
test services_gdrive::test_blocking_create_dir_existing           ... ok
test services_gdrive::test_blocking_remove_one_file               ... ok
test services_gdrive::test_list_dir_with_metakey                  ... ok
test services_gdrive::test_list_dir_with_metakey_complete         ... ok
test services_gdrive::test_blocking_read_full                     ... ok
test services_gdrive::test_write_with_dir_path                    ... ok
test services_gdrive::test_write_with_cache_control               ... ok
test services_gdrive::test_write_with_content_type                ... ok
test services_gdrive::test_write_with_content_disposition         ... ok
test services_gdrive::test_rename_source_dir                      ... ok
test services_gdrive::test_list_dir                               ... ok
test services_gdrive::test_list_sub_dir                           ... ok
test services_gdrive::test_rename_self                            ... ok
test services_gdrive::test_create_dir                             ... ok
test services_gdrive::test_stat_with_if_match                     ... ok
test services_gdrive::test_stat_with_if_none_match                ... ok
test services_gdrive::test_stat_root                              ... ok
test services_gdrive::test_stat_not_exist                         ... ok
test services_gdrive::test_read_range                             ... ok
test services_gdrive::test_read_large_range                       ... ok
test services_gdrive::test_reader_range                           ... ok
test services_gdrive::test_reader_from                            ... ok
test services_gdrive::test_reader_tail                            ... ok
test services_gdrive::test_read_not_exist                         ... ok
test services_gdrive::test_read_with_if_match                     ... ok
test services_gdrive::test_read_with_if_none_match                ... ok
test services_gdrive::test_fuzz_reader_with_range                 ... ok
test services_gdrive::test_fuzz_offset_reader                     ... ok
test services_gdrive::test_fuzz_part_reader                       ... ok
test services_gdrive::test_write_only                             ... ok
test services_gdrive::test_create_dir_existing                    ... ok
test services_gdrive::test_read_with_override_cache_control       ... ok
test services_gdrive::test_read_with_override_content_disposition ... ok
test services_gdrive::test_read_with_override_content_type        ... ok
test services_gdrive::test_write_with_special_chars               ... ok
test services_gdrive::test_stat_file                              ... ok
test services_gdrive::test_stat_with_special_chars                ... ok
test services_gdrive::test_stat_dir                               ... ok
test services_gdrive::test_stat_not_cleaned_path                  ... ok
test services_gdrive::test_delete_not_existing                    ... ok
test services_gdrive::test_writer_write                           ... ok
test services_gdrive::test_writer_sink                            ... ok
test services_gdrive::test_writer_copy                            ... ok
test services_gdrive::test_writer_abort                           ... ok
test services_gdrive::test_writer_futures_copy                    ... ok
test services_gdrive::test_fuzz_unsized_writer                    ... ok
test services_gdrive::test_rename_target_dir                      ... ok
test services_gdrive::test_fuzz_issue_2717                        ... ok
test services_gdrive::test_rename_file                            ... ok
test services_gdrive::test_delete_empty_dir                       ... ok
test services_gdrive::test_read_with_dir_path                     ... ok
test services_gdrive::test_read_full                              ... ok
test services_gdrive::test_delete_file                            ... ok
test services_gdrive::test_rename_nested                          ... ok
test services_gdrive::test_list_nested_dir                        ... ok
test services_gdrive::test_remove_one_file                        ... ok
test services_gdrive::test_read_with_special_chars                ... ok
test services_gdrive::test_delete_with_special_chars              ... ok
thread '<unnamed>' panicked at 'service doesn't support list hierarchy, it must be a bug', core/src/raw/oio/page/into_flat_page.rs:31:9
test services_gdrive::test_scan                                   ... FAILED
test services_gdrive::test_rename_overwrite                       ... ok
thread '<unnamed>' panicked at 'service doesn't support list hierarchy, it must be a bug', core/src/raw/oio/page/into_flat_page.rs:31:9
test services_gdrive::test_remove_all                             ... FAILED
test services_gdrive::test_invalid_reader_seek                    ... ok
test services_gdrive::test_delete_stream                          ... ok

failures:

---- services_gdrive::test_list_non_exist_dir ----
NotFound (permanent) at Pager::next, context: { service: gdrive, path: eb0dbc4d-116d-42c3-b72f-7d9111a8855a/ } => path not found: eb0dbc4d-116d-42c3-b72f-7d9111a8855a

---- services_gdrive::test_list_rich_dir ----
test panicked: create must succeed: PermissionDenied (permanent) at Writer::write => {
  "error": {
    "code": 403,
    "message": "User rate limit exceeded.",
    "errors": [
      {
        "message": "User rate limit exceeded.",
        "domain": "usageLimits",
        "reason": "userRateLimitExceeded"
      }
    ]
  }
}


Context:
   uri: https://www.googleapis.com/upload/drive/v3/files?uploadType=multipart
   response: Parts { status: 403, version: HTTP/2.0, headers: {"x-guploader-uploadid": "ADPycduFmdTm-n4wnDKTPIohkINp84FGAaJyB5XYskSAsrAmph40yDTFy_SS2SbBrVKQsDe9FxBYYpFBC0S3gLHroOENexDSw8RH", "cache-control": "private, max-age=0", "content-type": "application/json; charset=UTF-8", "date": "Thu, 07 Sep 2023 09:59:31 GMT", "expires": "Thu, 07 Sep 2023 09:59:31 GMT", "server": "ESF", "vary": "Origin, X-Origin", "x-content-type-options": "nosniff", "x-frame-options": "SAMEORIGIN", "x-xss-protection": "0", "content-length": "243", "alt-svc": "h3=\":443\"; ma=2592000,h3-29=\":443\"; ma=2592000"} }
   service: gdrive
   path: test_list_rich_dir/file-50


---- services_gdrive::test_scan_root ----
test panicked: service doesn't support list hierarchy, it must be a bug

---- services_gdrive::test_scan ----
test panicked: service doesn't support list hierarchy, it must be a bug

---- services_gdrive::test_remove_all ----
test panicked: service doesn't support list hierarchy, it must be a bug


failures:
    services_gdrive::test_list_non_exist_dir
    services_gdrive::test_list_rich_dir
    services_gdrive::test_scan_root
    services_gdrive::test_scan
    services_gdrive::test_remove_all

test result: FAILED. 81 passed; 5 failed; 0 ignored; 0 measured; 0 filtered out; finished in 405.37s

@Xuanwo
Copy link
Member

Xuanwo commented Sep 10, 2023

some failed test are because of google drive api User rate limit exceeded, maybe we can use retry layer in that test cases?

We have already enabled retry layer for all services. The thing that gdrive service should do is mapping userRateLimitExceeded error to ErrorKind::RateLimted and set temporary to true.

Please take s3's error handling as an example.

And some failed because of thread '<unnamed>' panicked at 'service doesn't support list hierarchy, it must be a bug', any suggestion?

Please check service's capability, list_with_delimiter_slash should be enabled.

@Young-Flash
Copy link
Member Author

all tests passed now 😄

running 86 tests
test services_gdrive::test_blocking_read_large_range              ... ok
test services_gdrive::test_blocking_fuzz_range_reader             ... ok
test services_gdrive::test_blocking_fuzz_offset_reader            ... ok
test services_gdrive::test_blocking_fuzz_part_reader              ... ok
test services_gdrive::test_blocking_read_range                    ... ok
test services_gdrive::test_blocking_write_with_dir_path           ... ok
test services_gdrive::test_blocking_read_not_exist                ... ok
test services_gdrive::test_blocking_stat_not_exist                ... ok
test services_gdrive::test_check                                  ... ok
test services_gdrive::test_list_non_exist_dir                     ... ok
test services_gdrive::test_blocking_create_dir                    ... ok
test services_gdrive::test_blocking_stat_dir                      ... ok
test services_gdrive::test_list_dir_with_file_path                ... ok
test services_gdrive::test_list_with_start_after                  ... ok
test services_gdrive::test_blocking_remove_one_file               ... ok
test services_gdrive::test_blocking_delete_file                   ... ok
test services_gdrive::test_blocking_write_with_special_chars      ... ok
test services_gdrive::test_list_empty_dir                         ... ok
test services_gdrive::test_blocking_stat_with_special_chars       ... ok
test services_gdrive::test_list_dir                               ... ok
test services_gdrive::test_blocking_stat_file                     ... ok
test services_gdrive::test_blocking_read_full                     ... ok
test services_gdrive::test_blocking_write_file                    ... ok
test services_gdrive::test_blocking_create_dir_existing           ... ok
test services_gdrive::test_list_dir_with_metakey                  ... ok
test services_gdrive::test_rename_non_existing_source             ... ok
test services_gdrive::test_list_sub_dir                           ... ok
test services_gdrive::test_write_with_dir_path                    ... ok
test services_gdrive::test_list_dir_with_metakey_complete         ... ok
test services_gdrive::test_write_with_cache_control               ... ok
test services_gdrive::test_write_with_content_type                ... ok
test services_gdrive::test_write_with_content_disposition         ... ok
test services_gdrive::test_rename_source_dir                      ... ok
test services_gdrive::test_create_dir                             ... ok
test services_gdrive::test_create_dir_existing                    ... ok
test services_gdrive::test_stat_dir                               ... ok
test services_gdrive::test_rename_self                            ... ok
test services_gdrive::test_stat_with_if_match                     ... ok
test services_gdrive::test_stat_with_if_none_match                ... ok
test services_gdrive::test_stat_root                              ... ok
test services_gdrive::test_stat_not_exist                         ... ok
test services_gdrive::test_read_range                             ... ok
test services_gdrive::test_read_large_range                       ... ok
test services_gdrive::test_reader_range                           ... ok
test services_gdrive::test_reader_from                            ... ok
test services_gdrive::test_reader_tail                            ... ok
test services_gdrive::test_read_not_exist                         ... ok
test services_gdrive::test_read_with_if_match                     ... ok
test services_gdrive::test_read_with_if_none_match                ... ok
test services_gdrive::test_fuzz_reader_with_range                 ... ok
test services_gdrive::test_fuzz_offset_reader                     ... ok
test services_gdrive::test_fuzz_part_reader                       ... ok
test services_gdrive::test_write_only                             ... ok
test services_gdrive::test_stat_file                              ... ok
test services_gdrive::test_read_with_override_cache_control       ... ok
test services_gdrive::test_read_with_override_content_disposition ... ok
test services_gdrive::test_read_with_override_content_type        ... ok
test services_gdrive::test_write_with_special_chars               ... ok
test services_gdrive::test_rename_file                            ... ok
test services_gdrive::test_rename_target_dir                      ... ok
test services_gdrive::test_scan_root                              ... ok
test services_gdrive::test_delete_not_existing                    ... ok
test services_gdrive::test_stat_with_special_chars                ... ok
test services_gdrive::test_writer_write                           ... ok
test services_gdrive::test_writer_sink                            ... ok
test services_gdrive::test_writer_copy                            ... ok
test services_gdrive::test_writer_abort                           ... ok
test services_gdrive::test_writer_futures_copy                    ... ok
test services_gdrive::test_fuzz_unsized_writer                    ... ok
test services_gdrive::test_list_nested_dir                        ... ok
test services_gdrive::test_fuzz_issue_2717                        ... ok
test services_gdrive::test_stat_not_cleaned_path                  ... ok
test services_gdrive::test_read_with_dir_path                     ... ok
test services_gdrive::test_delete_empty_dir                       ... ok
test services_gdrive::test_delete_file                            ... ok
test services_gdrive::test_rename_nested                          ... ok
test services_gdrive::test_read_full                              ... ok
test services_gdrive::test_remove_one_file                        ... ok
test services_gdrive::test_scan                                   ... ok
test services_gdrive::test_delete_with_special_chars              ... ok
test services_gdrive::test_invalid_reader_seek                    ... ok
test services_gdrive::test_read_with_special_chars                ... ok
test services_gdrive::test_rename_overwrite                       ... ok
test services_gdrive::test_remove_all                             ... ok
test services_gdrive::test_list_rich_dir                          ... ok
test services_gdrive::test_delete_stream                          ... ok

test result: ok. 86 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 382.48s

Xuanwo
Xuanwo approved these changes Sep 11, 2023
View reviewed changes
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.

Thanks!

@Xuanwo Xuanwo merged commit 57581b9 into apache:main Sep 11, 2023
@Young-Flash Young-Flash deleted the gdrive-list branch September 11, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

Assignees
No one assigned
Labels
releases-note/feat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Young-Flash @Xuanwo