You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "suyanhanx (via GitHub)" <gi...@apache.org> on 2023/05/26 11:22:19 UTC
[GitHub] [incubator-opendal] suyanhanx opened a new pull request, #2334: feat(core): test for append to file not created by append
suyanhanx opened a new pull request, #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334
(no comment)
--
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 #2334: feat(core): test for append to file not created by append
Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206696684
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
This is not to support. Just a capability to represent and enable test. :thinking:
--
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 #2334: feat(core): test for append to file not created by append
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206709909
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
No, I will reject this PR.
--
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 #2334: feat(core): test for append to file not created by append
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206689378
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
No, we should not have this.
--
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 #2334: feat(core): test for append to file not created by append
Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#issuecomment-1564318039
It doesn't matter.
--
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 #2334: feat(core): test for append to file not created by append
Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206709419
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
This is a promise, isn't it? 🤨
Only having this capability means that this service can append to normal files.
--
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 #2334: feat(core): test for append to file not created by append
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206692387
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
You are right, but we don't need to support.
--
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 #2334: feat(core): test for append to file not created by append
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206689378
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
No, we should not have this. We will not support append on non-appendable file. They are undefined behaviour.
--
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 #2334: feat(core): test for append to file not created by append
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206704574
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
> This is not to support. Just a capability to represent and enable test. thinking
No, there are two problems here:
- Capability is our public API. It cannot be added solely for testing purposes.
- Behavior test is our promise. Why should we need to add such *unsupported* things?
--
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 #2334: feat(core): test for append to file not created by append
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#issuecomment-1564317214
Thanks, but I don't like this idea.
--
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 #2334: feat(core): test for append to file not created by append
Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206691379
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
Some services do not distinguish between file types.
--
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 closed pull request #2334: feat(core): test for append to file not created by append
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo closed pull request #2334: feat(core): test for append to file not created by append
URL: https://github.com/apache/incubator-opendal/pull/2334
--
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 #2334: feat(core): test for append to file not created by append
Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #2334:
URL: https://github.com/apache/incubator-opendal/pull/2334#discussion_r1206691379
##########
core/src/types/capability.rs:
##########
@@ -88,6 +88,8 @@ pub struct Capability {
/// If operator supports append natively, it will be true.
pub append: bool,
+ /// If operator supports append to a file not created by append natively, it will be true.
+ pub append_normal: bool,
Review Comment:
Some services do not distinguish between file types, like `fs`, `sftp`.
--
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