You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "silver-ymz (via GitHub)" <gi...@apache.org> on 2023/05/23 10:05:48 UTC

[GitHub] [incubator-opendal] silver-ymz opened a new pull request, #2297: feat(services/sftp): add append support for sftp

silver-ymz opened a new pull request, #2297:
URL: https://github.com/apache/incubator-opendal/pull/2297

   Updated:
     - add append support for sftp
   
   In the tests, we will receive warning `opendal::layers::complete: appender has not been closed, must be a bug`. It also exists in [github action test](https://github.com/apache/incubator-opendal/actions/runs/5055444931/jobs/9071661273#step:4:546) for `fs` implementation. Should we concern about it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on pull request #2297: feat(services/sftp): add append support for sftp

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2297:
URL: https://github.com/apache/incubator-opendal/pull/2297#issuecomment-1558969817

   > Should we concern about it?
   
   Yes, it's a bug. cc @suyanhanx, we should close the appender by hand.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] suyanhanx commented on pull request #2297: feat(services/sftp): add append support for sftp

Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on PR #2297:
URL: https://github.com/apache/incubator-opendal/pull/2297#issuecomment-1558983752

   > > Should we concern about it?
   > 
   > Yes, it's a bug. cc @suyanhanx, we should close the appender by hand.
   
   fixed in #2298 🥲 This is due to incorrect detection.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2297: feat(services/sftp): add append support for sftp

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2297:
URL: https://github.com/apache/incubator-opendal/pull/2297#discussion_r1202052001


##########
core/src/services/sftp/writer.rs:
##########
@@ -53,3 +53,16 @@ impl oio::Write for SftpWriter {
         Ok(())
     }
 }
+
+#[async_trait]
+impl oio::Append for SftpWriter {

Review Comment:
   LGTM



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo merged pull request #2297: feat(services/sftp): add append support for sftp

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #2297:
URL: https://github.com/apache/incubator-opendal/pull/2297


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] suyanhanx commented on a diff in pull request #2297: feat(services/sftp): add append support for sftp

Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #2297:
URL: https://github.com/apache/incubator-opendal/pull/2297#discussion_r1202024041


##########
core/src/services/sftp/writer.rs:
##########
@@ -53,3 +53,16 @@ impl oio::Write for SftpWriter {
         Ok(())
     }
 }
+
+#[async_trait]
+impl oio::Append for SftpWriter {

Review Comment:
   Is such reuse appropriate? @Xuanwo 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org