feat(layers/blocking): add blocking layer

[yah01]

• Auto add blocking layer for backends which don't support blocking APIs
  fix feat: Providing Blocking API support for HTTP based services like S3 #2678

[Xuanwo]

Great!

[yah01]

Hi @Xuanwo The sftp related tests failed, but I don't get it how this occurs, I did clone the Handle from RUNTIME, what's the diff between sftp and the other backends, could you help on this?

[Xuanwo]

what's the diff between sftp and the other backends, could you help on this?

cc @silver-ymz, do you have any ideas?

[silver-ymz]

It seems that the dropping of sftp client does some magical things.

thread '<unnamed>' panicked at 'there is no reactor running, must be called from the context of a Tokio 1.x runtime', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/openssh-sftp-client-0.13.6/src/file/tokio_compat_file.rs:763:9

[Xuanwo]

It seems that the dropping of sftp client does some magical things.

Also cc @NobodyXu for some ideas.

[NobodyXu]

It seems that the dropping of sftp client does some magical things.

thread '<unnamed>' panicked at 'there is no reactor running, must be called from the context of a Tokio 1.x runtime', /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/openssh-sftp-client-0.13.6/src/file/tokio_compat_file.rs:763:9

TokioCompatFile::drop calls tokio::spawn to wait on all write_futures and log the errors if any.

We can change it to only do that if a tokio::Runtime is available.

[NobodyXu]

openssh-sftp-client v0.13.7 has released, which should fix this.

[Xuanwo]

openssh-sftp-client v0.13.7 has released, which should fix this.

Great! Thanks a lot!

[yah01]

Hi @Xuanwo, the sftp problem and the BlockingReader problem are blocking this PR, just wonder whether you are working on them? I could resolve them if you haven't started (should I create two new PRs?)

[NobodyXu]

Hi @Xuanwo, the sftp problem and the BlockingReader problem are blocking this PR, just wonder whether you are working on them? I could resolve them if you haven't started (should I create two new PRs?)

For the openssh-sftp-client bug, a simple bump is required, IMO you can open a new PR for that (or simply bump it in this PR if you prefer to do that here).

[Xuanwo]

I have fixed the seek support for blocking reader at: #2799

[Xuanwo]

Hi, @yah01, it's better not to do force push: OpenDAL always squash commits while merging so there is no need to do that.

[Xuanwo]

I'm guessing we can add following unit tests in blocking layer:

• init in blocking context, drop in blocking context
• init in blocking context, drop in async context
• init in async context, drop in blocking context
• init in async context, drop in async context

[Xuanwo]

I'm guessing we can add following unit tests in blocking layer:

• init in blocking context, drop in blocking context
• init in blocking context, drop in async context
• init in async context, drop in blocking context
• init in async context, drop in async context

This seems not a blocker here, we can improve this in later PRs.

[yah01]

@Xuanwo @NobodyXu It seems the sftp client is still in trouble, any idea?

[Xuanwo]

@Xuanwo @NobodyXu It seems the sftp client is still in trouble, any idea?

https://github.com/openssh-rust/openssh-sftp-client/blob/main/src/handle.rs#L22C15-L59

Seems OwnedHandle should also need some work.

[yah01]

Hi @NobodyXu , could you release a new version of openssh-sftp-client? Then we can use it for this, I'm going to bump this directly in this PR to make sure it works

[NobodyXu]

Hi @NobodyXu , could you release a new version of openssh-sftp-client? Then we can use it for this, I'm going to bump this directly in this PR to make sure it works

@silver-ymz has released openssh-sftp-client v0.13.8

[Xuanwo]

has released openssh-sftp-client v0.13.8

@NobodyXu, you are so awesome!

[NobodyXu]

Thank you, but @silver-ymz is the one who fixed the <OwnedHandle as Drop>::drop.

[Xuanwo]

Thank you, but @silver-ymz is the one who fixed the <OwnedHandle as Drop>::drop.

Fantastic! I'm thrilled to have @NobodyXu and @silver-ymz as part of our community!

[Xuanwo]

Thanks a lot for the work!

• @yah01 for adding blocking layer.
• @NobodyXu & @silver-ymz for fixing sftp related issues.

We will need some benchmark for blocking layer's perf and make sure it works on different context. But this PR is already good enough to get merged. Let's rocks!

[Xuanwo]

nextcloud failed for file locked, we should address it by add redis cache for it. Not related with this PR.

[yah01]

Thanks for you guys @Xuanwo @silver-ymz @NobodyXu, it's impossible to get this work done well without your help!

[chitralverma]

@yah01 amazing, couple of questions regarding this,

• Are there any user-facing examples to use the blocking API?
• Which services now support/ don't support blocking API?
• Is there a separate PR required to update the docs.rs with service-level capabilities? Since this will be part of the next release

[Xuanwo]

• Are there any user-facing examples to use the blocking API?

Yes, docs provided at BlockingOperator. This PR doesn't change our public API.

Which services now support/ don't support blocking API?

By adding blocking layers, services that only provide async API can have blocking API now.

Is there a separate PR required to update the docs.rs with service-level capabilities? Since this will be part of the next release

This question is related to #2790. I expect to address them before next release.