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

Make LocalFileSystem an Optional Feature #6055

Open
tustvold opened this issue Jul 15, 2024 · 5 comments · May be fixed by #6636
Open

Make LocalFileSystem an Optional Feature #6055

tustvold opened this issue Jul 15, 2024 · 5 comments · May be fixed by #6636
Labels
enhancement Any new improvement worthy of a entry in the changelog object-store Object Store Interface

Comments

@tustvold
Copy link
Contributor

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

Currently LocalFileSystem is always provided, and this in turn bring in a dependency on tokio and walkdir. There are scenarios were users may not need LocalFileSystem support and/or may not wish to link in tokio.

Describe the solution you'd like

Add an fs feature flag that is enabled by default and feature gates LocalFilesystem and its dependencies. It is unclear if this would constitute a breaking change, and so it might need to wait for a breaking release

Describe alternatives you've considered

Additional context
@tustvold tustvold added the enhancement Any new improvement worthy of a entry in the changelog label Jul 15, 2024
@tustvold tustvold mentioned this issue Jul 15, 2024
Open
@alamb
Copy link
Contributor

alamb commented Jul 15, 2024

makes sense to me

@jiacai2050
Copy link
Contributor

Hi, after search(query prompt: use tokio) the source code, I found there is no tokio dependency in local.rs

./*ivy-occur counsel-rg "tokio"*:8:./src/client/token.rs:20:use tokio::sync::Mutex;
./*ivy-occur counsel-rg "tokio"*:10:./src/buffered.rs:33:use tokio::io::{AsyncBufRead, AsyncRead, AsyncSeek, AsyncWrite, ReadBuf};
./*ivy-occur counsel-rg "tokio"*:13:./src/buffered.rs:477:    use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncSeekExt, AsyncWriteExt};
./*ivy-occur counsel-rg "tokio"*:24:./src/client/mock_server.rs:31:use tokio::net::TcpListener;
./*ivy-occur counsel-rg "tokio"*:25:./src/client/mock_server.rs:32:use tokio::sync::oneshot;
./*ivy-occur counsel-rg "tokio"*:26:./src/client/mock_server.rs:33:use tokio::task::{JoinHandle, JoinSet};
./*ivy-occur counsel-rg "tokio"*:46:./src/throttle.rs:403:    use tokio::time::Duration;
./*ivy-occur counsel-rg "tokio"*:47:./src/throttle.rs:404:    use tokio::time::Instant;
./*ivy-occur counsel-rg "tokio"*:56:./src/upload.rs:25:use tokio::task::JoinSet;
./*ivy-occur counsel-rg "tokio"*:71:./src/local.rs:219:/// a tokio context, this will use [`tokio::runtime::Handle::spawn_blocking`] to dispatch
./*ivy-occur counsel-rg "tokio"*:101:./src/lib.rs:278://! # use tokio::io::AsyncWriteExt;
./*ivy-occur counsel-rg "tokio"*:102:./src/lib.rs:307://! # use tokio::io::AsyncWriteExt;
./*ivy-occur counsel-rg "tokio"*:103:./src/lib.rs:339://! # use tokio::io::AsyncWriteExt;
./*ivy-occur counsel-rg "tokio"*:104:./src/lib.rs:378://! # use tokio::io::AsyncWriteExt;
./*ivy-occur counsel-rg "tokio"*:105:./src/lib.rs:445://! # use tokio::io::AsyncWriteExt;
./*ivy-occur counsel-rg "tokio"*:109:./src/lib.rs:1082:    /// a tokio context, this will use [`tokio::runtime::Handle::spawn_blocking`] to dispatch
./*ivy-occur counsel-rg "tokio"*:113:./src/lib.rs:1295:    use tokio::io::AsyncWriteExt;
./*ivy-occur counsel-rg "tokio"*:124:./src/limit.rs:32:use tokio::sync::{OwnedSemaphorePermit, Semaphore};
./*ivy-occur counsel-rg "tokio"*:125:./src/limit.rs:283:    use tokio::time::timeout;

@alamb
Copy link
Contributor

alamb commented Jul 17, 2024 •
edited
Loading

Here are a few:

arrow-rs/object_store/src/local.rs

Line 564 in 199ce91

if tokio::runtime::Handle::try_current().is_err() {

arrow-rs/object_store/src/local.rs

Line 846 in 199ce91

match tokio::runtime::Handle::try_current() {

Github search fu suggests there are 32 uses: https://github.com/search?q=repo%3Aapache%2Farrow-rs+tokio+path%3A%2F%5Eobject_store%5C%2F%2F+path%3A%2F%5Eobject_store%5C%2Fsrc%5C%2Flocal%2F&type=code

Screenshot 2024-07-17 at 5 42 01 AM

@jiacai2050
Copy link
Contributor

Thanks for pointing out.

If fs feature in enabled by default, this is not a breaking change for users?

@tustvold
Copy link
Contributor Author

People could have disabled default features and it would be a breaking change for them

@alamb alamb added the object-store Object Store Interface label Jul 18, 2024
@tustvold tustvold mentioned this issue Oct 21, 2024
Open
2 tasks
@Turbo87 Turbo87 linked a pull request Oct 28, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement Any new improvement worthy of a entry in the changelog object-store Object Store Interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

object_store: Add enabled-by-default "fs" feature Turbo87/arrow-rs
3 participants
@alamb @tustvold @jiacai2050