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

Add GetOptions::head #4931

Merged
merged 1 commit into from
Oct 15, 2023
Merged

Add GetOptions::head #4931

merged 1 commit into from
Oct 15, 2023

Conversation

tustvold
Copy link
Contributor

Which issue does this PR close?

Relates to #4925
Relates to #4525

Rationale for this change

Adds GetOptions::head. This achieves a couple of goals:

  • Allows conditional HEAD requests (Get Object By Version #4925)
  • Allows a default impl of ObjectStore::head
  • Provides a mechanism to get a local file if available, without the risk of performing a network fetch of the entire object

What changes are included in this PR?

Are there any user-facing changes?

@tustvold tustvold added the api-change Changes to the arrow API label Oct 13, 2023
@github-actions github-actions bot added the object-store Object Store Interface label Oct 13, 2023
});

let location = Path::from(filename);
integration.head(&location).await.unwrap();
Copy link
Contributor Author

@tustvold tustvold Oct 13, 2023

Choose a reason for hiding this comment

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

This has to be moved after the spawn_blocking as it now will open the file, which will block until the writer opens it, I don't see this being an issue as the behaviour of a head request on a file that gets mutated behind the scenes are not exactly very well defined anyway 😅

// Need to open read and write side in parallel
let spawned = tokio::task::spawn_blocking(|| {
OpenOptions::new().write(true).open(path).unwrap();
Copy link
Contributor Author

@tustvold tustvold Oct 13, 2023

Choose a reason for hiding this comment

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

This semicolon fixes a subtle bug, where the writer is immediately dropped which then causes subsequent opens to block. I'm actually somewhat surprised it has worked semi-reliably so far. By returning the File, we defer destruction until after we're done with the file

@tustvold tustvold merged commit bb8e42f into apache:master Oct 15, 2023
13 checks passed
@tustvold tustvold mentioned this pull request Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change Changes to the arrow API object-store Object Store Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants