You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/03/03 06:51:00 UTC
[GitHub] [iceberg] dylzlt opened a new pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
dylzlt opened a new pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258
I add the usage in javadoc of ValidationException.
Can you give me some suggestion if my description is not accurate.
thanks! @jackye1995
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1067155693
I accidentally mis-labelled it as Core from Api when simplifying the commit message, sorry about 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on a change in pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on a change in pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#discussion_r821296252
##########
File path: api/src/main/java/org/apache/iceberg/exceptions/ValidationException.java
##########
@@ -24,7 +24,8 @@
import org.apache.iceberg.Schema;
/**
- * Exception raised when validation checks fail.
+ * Exception raised when arguments inconsistencies checks fail.
+ * {@link IllegalArgumentException} raised when arguments aren't allowed.
Review comment:
ok, I have modified that.Thank you for your review
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on a change in pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on a change in pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#discussion_r824416909
##########
File path: api/src/main/java/org/apache/iceberg/exceptions/ValidationException.java
##########
@@ -24,7 +24,11 @@
import org.apache.iceberg.Schema;
/**
- * Exception raised when validation checks fail.
+ * Exception which is raised when the arguments are valid in isolation,
Review comment:
Small nit, can you try to make each line be longer and try to line up towards the right?
I don't see many places where the doc line-breaks on a comma, usually it's on a period. You can also add a <p> after the period.
A good example in my opinion : https://github.com/apache/iceberg/blob/master/api/src/main/java/org/apache/iceberg/ExpireSnapshots.java
Thank you
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1067151701
Thanks @dylzlt for the change and @RussellSpitzer for additional review
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1060934197
> But there is a problem that the IsolationLevel is in iceberg-core module rather than api(the link is invalid)
Yea I think you can qualify the class? Or else removing it is also fine with me, as it's not a big deal.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on a change in pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on a change in pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#discussion_r824416909
##########
File path: api/src/main/java/org/apache/iceberg/exceptions/ValidationException.java
##########
@@ -24,7 +24,11 @@
import org.apache.iceberg.Schema;
/**
- * Exception raised when validation checks fail.
+ * Exception which is raised when the arguments are valid in isolation,
Review comment:
Small nit, can you try to make each line be longer and try to line up towards the right?
I don't see many places where the doc line-breaks on a comma, usually it's on a period. You can also add a ```<p>``` after the period.
A good example in my opinion : https://github.com/apache/iceberg/blob/master/api/src/main/java/org/apache/iceberg/ExpireSnapshots.java
Thank you
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho merged pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho merged pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1060154089
>
OK, I agree that the description is better!
But there is a problem that the IsolationLevel is in iceberg-core module rather than api(the link is invalid), so remove the link will be better?
the example is here:
“For example, this is thrown when attempting to create a table with a {@link PartitionSpec} that is not compatible with the table {@link Schema}, or attempting to add or remove files to a table that would violate the configured IsolationLevel due to concurrent commits.”
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#discussion_r820931190
##########
File path: api/src/main/java/org/apache/iceberg/exceptions/ValidationException.java
##########
@@ -24,7 +24,8 @@
import org.apache.iceberg.Schema;
/**
- * Exception raised when validation checks fail.
+ * Exception raised when arguments inconsistencies checks fail.
Review comment:
"Raised when the arguments are valid in isolation, but not in conjunction with other arguments or state"?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho edited a comment on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho edited a comment on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1059416924
I think it needs a bit more context, how about something like:
"Exception raised when validation checks fail. A ValidationException occurs from inconsistent state that causes an operation to abort, as opposed to an {@link IllegalArgumentException}, which occurs from invalid arguments."
I would also love to add another example here:
"For example, this is thrown when attempting to create a table with a {@link PartitionSpec} that is not compatible with the table {@link Schema}, or attempting to add or remove files to a table that would violate the configured {@link IsolationLevel} due to concurrent commits."
That being said, I did not raise the original issue , so maybe @rdblue @jackye1995 have more thoughts, or @aokolnychyi @RussellSpitzer @kbendick as well :)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho edited a comment on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho edited a comment on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1059416924
I think it needs a bit more context, how about something like:
"Exception raised when validation checks fail. A ValidationException occurs from inconsistent state that causes an operation to abort, as opposed to an {@link IllegalArgumentException}, which occurs from invalid arguments."
I would also love to add another example here:
"For example, this is thrown when attempting to create a table with a {@link PartitionSpec} that is not compatible with the table {@link Schema}, or attempting to add or remove files to a table that would violate the configured isolation level due to concurrent commits."
That being said, I did not raise the original issue , so maybe @rdblue @jackye1995 have more thoughts, or @aokolnychyi @RussellSpitzer @kbendick as well :)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1059416924
I think it needs a bit more context, how about something like:
"Exception raised when validation checks fail. A ValidationException occurs from inconsistent state that would cause an operation to abort, as opposed to an {@link IllegalArgumentException}, which occurs from invalid arguments."
I would also love to add another example here:
"For example, this is thrown when attempting to create a table with a {@link PartitionSpec} that is not compatible with the table {@link Schema}, or attempting to add or remove files to a table that would violate the configured isolation level due to concurrent commits."
That being said, I did not raise the original issue , so maybe @rdblue @jackye1995 have more thoughts, or @aokolnychyi @RussellSpitzer @kbendick as well :)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt removed a comment on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt removed a comment on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1064793484
>
ok, I get it. Thank you for your patience
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1064689751
@dylzlt sorry for the delay, I synced with @RussellSpitzer to reduce the back and forth, his main concern was to make sure we say IllegalArgumentException means it is never valid as opposed to it's not valid now because of other arguments.
So can you do something like?
```
Exception which is raised when the arguments are valid in isolation, but not in conjunction with other arguments or state, as opposed to {@link IllegalArgumentException} which is raised when an argument value is always invalid. A ValidationException will cause the operation to abort.
```
I think it should be good to go after 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1064793484
>
ok, I get it. Thank you for your patience
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#discussion_r820931680
##########
File path: api/src/main/java/org/apache/iceberg/exceptions/ValidationException.java
##########
@@ -24,7 +24,8 @@
import org.apache.iceberg.Schema;
/**
- * Exception raised when validation checks fail.
+ * Exception raised when arguments inconsistencies checks fail.
+ * {@link IllegalArgumentException} raised when arguments aren't allowed.
Review comment:
Raised when this argument value would never be appropriate?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1064794777
> @dylzlt sorry for the delay, I synced with @RussellSpitzer to reduce the back and forth, his main concern was to make sure we say IllegalArgumentException means it is never valid as opposed to it's not valid now because of other arguments.
>
> So can you do something like?
>
> ```
> Exception which is raised when the arguments are valid in isolation, but not in conjunction with other arguments or state, as opposed to {@link IllegalArgumentException} which is raised when an argument value is always invalid. A ValidationException will cause the operation to abort.
> ```
>
> I think it should be good to go after this.
ok, I get it. Thank you for your patience. I complete 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1058395386
Not sure if we should put developer-facing comments here, as this is a user-facing Javadoc.. Maybe a better place is the Contribution guide? https://iceberg.apache.org/contribute/
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on a change in pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on a change in pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#discussion_r825550272
##########
File path: api/src/main/java/org/apache/iceberg/exceptions/ValidationException.java
##########
@@ -24,7 +24,11 @@
import org.apache.iceberg.Schema;
/**
- * Exception raised when validation checks fail.
+ * Exception which is raised when the arguments are valid in isolation,
Review comment:
ok, I‘m sorry for it.
I just want to make a part of sentence is complete in one line.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on a change in pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on a change in pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#discussion_r821296104
##########
File path: api/src/main/java/org/apache/iceberg/exceptions/ValidationException.java
##########
@@ -24,7 +24,8 @@
import org.apache.iceberg.Schema;
/**
- * Exception raised when validation checks fail.
+ * Exception raised when arguments inconsistencies checks fail.
Review comment:
Thank you for that. I have modified that.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1058869849
> Yea, that's what I am thinking, because this will be on Iceberg's user-facing Javadocs. Also in general we try to keep documentation free from personal pronouns (like we, I..) and just state what the class does.
Thank you very much for your explanation, I modify the doc, please take another look for me，thanks！
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1058784872
Yea, that's what I am thinking, because this will be on Iceberg's user-facing Javadocs. Also in general we try to keep documentation free from personal pronouns (like we, I..) and just state what the class does.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] dylzlt commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
dylzlt commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1058748283
> Sorry I just read the original issue as it wasnt linked (#3895 ) , I get it now and think would be great to have a Javadoc on this class.
>
> I think the problem is that this proposed Javadoc is developer-facing, and should be user-facing. Specifically, the 'we' here seems like its meant for developer to know what exception to throw... but it should be instead describing to user what situations to expect it from.
Oh, thanks for your suggestion. You mean the Javadoc is used to tell user arguments inconsistencies when they catch the ValidationException, rather than tell developer how to use it(because this is developer rule)?Is my understanding right?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho commented on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho commented on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1058686843
Sorry I just read the original issue as it wasnt linked (#3895 ) , I get it now and think would be great to have a Javadoc on this class.
I think the problem is that this proposed Javadoc is developer-facing, and should be user-facing. Specifically, the 'we' here seems like its meant for developer to know what exception to throw... but it should be instead describing to user what situations to expect it from.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho edited a comment on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho edited a comment on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1060934197
> But there is a problem that the IsolationLevel is in iceberg-core module rather than api(the link is invalid)
Yea I think you can fully-qualify the class name? Or else removing it is also fine with me, as it's not a big deal.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] szehon-ho edited a comment on pull request #4258: API:Add the usage in javadoc of ValidationException(#3895)
Posted by GitBox <gi...@apache.org>.
szehon-ho edited a comment on pull request #4258:
URL: https://github.com/apache/iceberg/pull/4258#issuecomment-1060934197
> But there is a problem that the IsolationLevel is in iceberg-core module rather than api(the link is invalid)
Yea I think you can fully-qualify the class name? Or else removing it is also fine with me, as it's not a big deal.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org