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

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
@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

Here are a few:

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any new improvement worthy of a entry in the changelog object-store Object Store Interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants