[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16498236#comment-16498236
]
Chao Sun commented on HDFS-13578:
-
Thanks [~xkrogen] and [~elgoiri] for the review!
> Add ReadOnly
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16498232#comment-16498232
]
Erik Krogen commented on HDFS-13578:
v6 patch looks good to me. I just committed it to branch
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494457#comment-16494457
]
genericqa commented on HDFS-13578:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494372#comment-16494372
]
Chao Sun commented on HDFS-13578:
-
Uploaded patch v6.
> Add ReadOnly annotation to methods in
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494369#comment-16494369
]
Chao Sun commented on HDFS-13578:
-
My bad. Completely forgot about the tests. :(
> Add ReadOnly
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494358#comment-16494358
]
genericqa commented on HDFS-13578:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem ||
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494351#comment-16494351
]
Erik Krogen commented on HDFS-13578:
v05 is looking very good, but a number of ops are missed from
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494170#comment-16494170
]
Chao Sun commented on HDFS-13578:
-
Attached patch v5.
> Add ReadOnly annotation to methods in
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494060#comment-16494060
]
Chao Sun commented on HDFS-13578:
-
Got it. Sorry I got confused on the UNCHECKED annotation. Will upload
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494034#comment-16494034
]
Erik Krogen commented on HDFS-13578:
I think we are unnecessarily confusing {{OperationCategory}}
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16494018#comment-16494018
]
Chao Sun commented on HDFS-13578:
-
Hmm... seems there are some other operations that are UNCHECKED and
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16493910#comment-16493910
]
Chao Sun commented on HDFS-13578:
-
{quote}For the datanode methods, does it matter that it is marked as
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16493821#comment-16493821
]
Íñigo Goiri commented on HDFS-13578:
bq. I would lean towards the latter because I think the
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16493811#comment-16493811
]
Erik Krogen commented on HDFS-13578:
{quote}
One thing I'm thinking is whether we should have tests
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16491411#comment-16491411
]
Íñigo Goiri commented on HDFS-13578:
bq. One thing I'm thinking is whether we should have tests for
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16491251#comment-16491251
]
Chao Sun commented on HDFS-13578:
-
Thanks [~xkrogen]!
1. Regarding methods need ReadOnly, you're right, I
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16490911#comment-16490911
]
Erik Krogen commented on HDFS-13578:
Thanks for the comments [~elgoiri]! I agree about adding the
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16490086#comment-16490086
]
Íñigo Goiri commented on HDFS-13578:
Thanks [~csun] for tackling my comments.
I'll let [~xkrogen]
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16490079#comment-16490079
]
genericqa commented on HDFS-13578:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16490023#comment-16490023
]
Chao Sun commented on HDFS-13578:
-
OK. Uploaded patch v4 to address the comments.
> Add ReadOnly
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489963#comment-16489963
]
Íñigo Goiri commented on HDFS-13578:
The annotation I think can be checked.
It's a static definition
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489937#comment-16489937
]
Chao Sun commented on HDFS-13578:
-
[~elgoiri]: I see what you mean. Can this be done by adding a notice to
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489923#comment-16489923
]
Íñigo Goiri commented on HDFS-13578:
bq. This will not work as whether to update atime is decided on
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489915#comment-16489915
]
Chao Sun commented on HDFS-13578:
-
Got it. Will make the change.
bq. Not sure of the wording of XXX but
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489908#comment-16489908
]
Íñigo Goiri commented on HDFS-13578:
You are currently doing:
{code}
69@Test
70public
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489823#comment-16489823
]
genericqa commented on HDFS-13578:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489692#comment-16489692
]
Chao Sun commented on HDFS-13578:
-
Uploaded patch v2 to fix style issues.
> Add ReadOnly annotation to
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16488091#comment-16488091
]
Chao Sun commented on HDFS-13578:
-
Thanks [~elgoiri]. Regarding:
{code}
for (Method m : allMethods) {
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16487876#comment-16487876
]
genericqa commented on HDFS-13578:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16487813#comment-16487813
]
Íñigo Goiri commented on HDFS-13578:
A minor nit, you could do:
{code}
for (String methodName :
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16487776#comment-16487776
]
Chao Sun commented on HDFS-13578:
-
Thanks [~elgoiri]: updated patch v1 with test cases.
> Add ReadOnly
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16486553#comment-16486553
]
Íñigo Goiri commented on HDFS-13578:
[~csun], not super useful to have tests for this but I would add
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16486528#comment-16486528
]
genericqa commented on HDFS-13578:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem ||
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16479379#comment-16479379
]
Chao Sun commented on HDFS-13578:
-
[~xkrogen]: good point. I think we should recommend people to disable
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16479290#comment-16479290
]
Íñigo Goiri commented on HDFS-13578:
Can you post an example of how will you check if the annotation
[
https://issues.apache.org/jira/browse/HDFS-13578?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16479148#comment-16479148
]
Erik Krogen commented on HDFS-13578:
One point I want to bring up is the case of
36 matches
Mail list logo
