You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "A-Stupid-Sun (via GitHub)" <gi...@apache.org> on 2023/06/04 17:30:51 UTC

[GitHub] [incubator-opendal] A-Stupid-Sun opened a new pull request, #2414: Fix oss batch max operations

A-Stupid-Sun opened a new pull request, #2414:
URL: https://github.com/apache/incubator-opendal/pull/2414

   related issue #2228 
   This pr add new batch_max_operations config for oss service and use default value (1000 )if not set.


-- 
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] A-Stupid-Sun commented on a diff in pull request #2414: fix(services/oss): Fix oss batch max operations

Posted by "A-Stupid-Sun (via GitHub)" <gi...@apache.org>.
A-Stupid-Sun commented on code in PR #2414:
URL: https://github.com/apache/incubator-opendal/pull/2414#discussion_r1217469006


##########
core/src/services/oss/backend.rs:
##########
@@ -354,6 +369,7 @@ impl Builder for OssBuilder {
                 server_side_encryption,
                 server_side_encryption_key_id,
                 write_min_size,
+                batch_max_operations,

Review Comment:
   I tried it, but not sure, please take a look



-- 
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 #2414: fix(services/oss): Fix oss batch max operations

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

   > But OSS doesn't need this setting.
   
   Since S3 has different implementations such as AWS S3, R2, etc., we require a configuration value for batch max operations. However, OSS only has one implementation which results in a stable batch max operation.


-- 
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] A-Stupid-Sun commented on pull request #2414: fix(services/oss): Fix oss batch max operations

Posted by "A-Stupid-Sun (via GitHub)" <gi...@apache.org>.
A-Stupid-Sun commented on PR #2414:
URL: https://github.com/apache/incubator-opendal/pull/2414#issuecomment-1575996122

   > Hi, Thanks for the PR! But OSS doesn't need this setting. Maybe you can contact with @manulpatel at PR #2354 to see if you can continue his work?
   
   sorry, i saw the issue #2228 which says s3, oss, a blob need this setting so i did this, should i close this or now?


-- 
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 #2414: fix(services/oss): Fix oss batch max operations

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

   Hi, Thanks for the PR! But OSS doesn't need this setting. Maybe you can contact with @manulpatel at PR https://github.com/apache/incubator-opendal/pull/2354 to see if you can continue his work?


-- 
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 #2414: fix(services/oss): Fix oss batch max operations

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


##########
core/src/services/oss/backend.rs:
##########
@@ -354,6 +369,7 @@ impl Builder for OssBuilder {
                 server_side_encryption,
                 server_side_encryption_key_id,
                 write_min_size,
+                batch_max_operations,

Review Comment:
   Please also set `batch_max_operations` in `Capability`



-- 
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 #2414: fix(services/oss): Fix oss batch max operations

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


-- 
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 #2414: fix(services/oss): Fix oss batch max operations

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

   > thanks for your help, i will close it now
   
   Hi, this PR is required. Let's get it merged!@


-- 
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 #2414: fix(services/oss): Fix oss batch max operations

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

   > sorry, i saw the issue #2228 which says s3, oss, a zblob need this setting so i did this, should i close this pr now?
   
   Your understanding is correct. I overlooked some context regarding this issue, and now I realize that this PR is necessary. Please accept my apologies for the mistake!


-- 
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] A-Stupid-Sun commented on pull request #2414: fix(services/oss): Fix oss batch max operations

Posted by "A-Stupid-Sun (via GitHub)" <gi...@apache.org>.
A-Stupid-Sun commented on PR #2414:
URL: https://github.com/apache/incubator-opendal/pull/2414#issuecomment-1575998323

   thanks for your help, i will close it now


-- 
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] A-Stupid-Sun commented on pull request #2414: fix(services/oss): Fix oss batch max operations

Posted by "A-Stupid-Sun (via GitHub)" <gi...@apache.org>.
A-Stupid-Sun commented on PR #2414:
URL: https://github.com/apache/incubator-opendal/pull/2414#issuecomment-1575997732

   > > But OSS doesn't need this setting.
   > 
   > Since S3 has different implementations such as AWS S3, R2, etc., we require a configuration value for batch max operations. However, OSS only has one implementation which results in a stable batch max operation.
   
   yes, i read the doc of oss, and don’t find this configuration 😥


-- 
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] A-Stupid-Sun commented on pull request #2414: fix(services/oss): Fix oss batch max operations

Posted by "A-Stupid-Sun (via GitHub)" <gi...@apache.org>.
A-Stupid-Sun commented on PR #2414:
URL: https://github.com/apache/incubator-opendal/pull/2414#issuecomment-1575999267

   > > thanks for your help, i will close it now
   > 
   > Hi, this PR is required. Let's get it merged!
   
   my pleasure, i will try my best


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