-
Notifications
You must be signed in to change notification settings - Fork 530
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(services/onedrive): List dir shows metadata #5632
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test workflow in .github/services
is missing, we should add one.
core/src/services/onedrive/lister.rs
Outdated
oio::Entry::new(&normalized_path, Metadata::new(EntryMode::FILE)) | ||
} | ||
let mut meta = Metadata::new(entry_mode) | ||
.with_last_modified( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have some time functions in chrono_utils.rs
, like: parse_datetime_from_rfc2822
. Maybe we could reuse them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tip, added.
I remembered that we failed to set up a OneDrive test. |
26c9d63
to
3e64864
Compare
I generated a token from a personal test account with Additional permissions are useful for API Graph explorer which I recommend:
What do you want me next? These actions look sensible:
|
3e64864
to
db4c88b
Compare
Since we don't have a test workflow for this, have you run the tests locally ? You can use the command |
Most my test folders have 0 in size, but one has 3. I didn't dig into it.
db4c88b
to
f370374
Compare
Rebased and ready. OPENDAL_TEST=onedrive cargo test behavior --features tests,services-onedrive -- --show-output
|
LGTM. Thanks for working on this. @erickguan |
@meteorgan happy to help! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @erickguan for working on this and thank you @meteorgan for the review.
By the way, @erickguan would you like to help fix the conflicts? |
Sure, I will rebase it and ping you. |
Which issue does this PR close?
Part of #4746.
What changes are included in this PR?
Supports OneDrive lister with metadata.