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

[BUG] Filter out size-0 directory marker files during s3 globs #1629

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

jaychia
Copy link
Contributor

@jaychia jaychia commented Nov 17, 2023

@github-actions github-actions bot added the bug Something isn't working label Nov 17, 2023
// However they can lead to unexpected globbing behavior since most users do not expect them to exist
|result_stream| {
result_stream
.filter_map(|fm| async move {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot for the life of me get .filter(...) to work! 😭

.filter receives fm as a reference, which seems to cause a whole bunch of lifetime issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved logic to shared fn glob utility, inside of the stream! so we don't have to use the StreamExt::filter function anymore

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Merging #1629 (a6a3d61) into main (427a4fe) will decrease coverage by 2.46%.
Report is 9 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1629      +/-   ##
==========================================
- Coverage   87.32%   84.86%   -2.46%     
==========================================
  Files          51       55       +4     
  Lines        5040     5274     +234     
==========================================
+ Hits         4401     4476      +75     
- Misses        639      798     +159     

see 10 files with indirect coverage changes

src/daft-io/src/object_store_glob.rs Outdated Show resolved Hide resolved
@jaychia jaychia enabled auto-merge (squash) November 17, 2023 06:16
@jaychia jaychia merged commit f18aeae into main Nov 17, 2023
38 of 39 checks passed
@jaychia jaychia deleted the jay/filter-size0-glob-results branch November 17, 2023 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants