[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587) URL: https://github.com/apache/hbase/pull/616#discussion_r326461597 ## File path: hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestOperation.java ## @@ -60,6 +59,9 @@ import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.BuilderStyleTest; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.GsonUtil; +import org.apache.hbase.thirdparty.com.google.common.reflect.TypeToken; Review comment: It has something to do with jdk7 compatibility or some issue in general? Sure, Let me take care of 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587) URL: https://github.com/apache/hbase/pull/616#discussion_r324038075 ## File path: hbase-testing-util/pom.xml ## @@ -164,6 +164,11 @@ + +org.codehaus.jackson Review comment: hbase-testing-util is used for running unit tests only by downstreamers? like just to bring up HBase minicluster by downstreamers and run their own tests on top of it? if no, then let me change this to `compile` Edit: I think better to include at `compile` given downstreames requirement 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587) URL: https://github.com/apache/hbase/pull/616#discussion_r324038075 ## File path: hbase-testing-util/pom.xml ## @@ -164,6 +164,11 @@ + +org.codehaus.jackson Review comment: hbase-testing-util is used for running unit tests only by downstreamers? like just to bring up HBase minicluster by downstreamers and run their own tests on top of it? if no, then let me change this to `compile` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587) URL: https://github.com/apache/hbase/pull/616#discussion_r324031522 ## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheUtil.java ## @@ -35,6 +30,13 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.metrics.impl.FastLongHistogram; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.GsonUtil; + + +import org.apache.hbase.thirdparty.com.google.gson.Gson; Review comment: hbase-server is getting it from hbase-common transitively, it should be fine? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587) URL: https://github.com/apache/hbase/pull/616#discussion_r324031028 ## File path: hbase-testing-util/pom.xml ## @@ -164,6 +164,11 @@ + +org.codehaus.jackson Review comment: without Jackson1, minicluster is not coming up, this is mostly because Hadoop requires Jackson1 and hence, just kept this dependency at provided scope 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587) URL: https://github.com/apache/hbase/pull/616#discussion_r324031028 ## File path: hbase-testing-util/pom.xml ## @@ -164,6 +164,11 @@ + +org.codehaus.jackson Review comment: without Jackson1, minicluster is not coming up, this is mostly because Hadoop requires Jackson1 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving
from Jackson2 to shaded Gson (Backport HBASE-20587)
URL: https://github.com/apache/hbase/pull/616#discussion_r323820422
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java
##
@@ -52,11 +51,9 @@
* This class is not thread safe.
*/
@InterfaceAudience.Private
-@JsonIgnoreProperties({"indexStatistics", "freeSize", "usedSize"})
Review comment:
Just verified the significant difference in JSON string for Gson and
Jackson2. Sure, will call out in release note.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving
from Jackson2 to shaded Gson (Backport HBASE-20587)
URL: https://github.com/apache/hbase/pull/616#discussion_r323793457
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java
##
@@ -52,11 +51,9 @@
* This class is not thread safe.
*/
@InterfaceAudience.Private
-@JsonIgnoreProperties({"indexStatistics", "freeSize", "usedSize"})
Review comment:
IndexStatistics is static class, freeSize is getter method: `public long
getFreeSize()` and usedSize is instance var of BucketAllocator and hence, only
that is made transient.
I am not sure how class and getter method can be prevented from
serialization as transient is not applied to them. Again, this change is inline
with branch-2.
Let me verify json with unit test
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving
from Jackson2 to shaded Gson (Backport HBASE-20587)
URL: https://github.com/apache/hbase/pull/616#discussion_r323793457
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java
##
@@ -52,11 +51,9 @@
* This class is not thread safe.
*/
@InterfaceAudience.Private
-@JsonIgnoreProperties({"indexStatistics", "freeSize", "usedSize"})
Review comment:
IndexStatistics is static class, freeSize is getter method: `public long
getFreeSize()` and usedSize is instance var of BucketAllocator and hence, only
that is made transient.
I am not sure how class and getter method can be prevented from
serialization as transient is not applied to them. Again, this change is inline
with branch-2
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving
from Jackson2 to shaded Gson (Backport HBASE-20587)
URL: https://github.com/apache/hbase/pull/616#discussion_r323776865
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/http/jmx/JMXJsonServlet.java
##
@@ -163,14 +163,14 @@ public void doGet(HttpServletRequest request,
HttpServletResponse response) thro
try {
jsonpcb = checkCallbackName(request.getParameter(CALLBACK_PARAM));
writer = response.getWriter();
-beanWriter = this.jsonBeanWriter.open(writer);
// "callback" parameter implies JSONP outpout
if (jsonpcb != null) {
response.setContentType("application/javascript; charset=utf8");
writer.write(jsonpcb + "(");
} else {
response.setContentType("application/json; charset=utf8");
}
+beanWriter = this.jsonBeanWriter.open(writer);
Review comment:
this is part of the backport
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] virajjasani commented on a change in pull request #616: HBASE-23015 : Moving from Jackson2 to shaded Gson (Backport HBASE-20587)
virajjasani commented on a change in pull request #616: HBASE-23015 : Moving
from Jackson2 to shaded Gson (Backport HBASE-20587)
URL: https://github.com/apache/hbase/pull/616#discussion_r323777027
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheUtil.java
##
@@ -102,15 +121,10 @@ public String getFilename() {
}
/**
- * @param filename
- * @param blocks
* @return A JSON String of filename and counts of
blocks
- * @throws JsonGenerationException
- * @throws JsonMappingException
- * @throws IOException
*/
- public static String toJSON(final String filename, final
NavigableSet blocks)
- throws JsonGenerationException, JsonMappingException, IOException {
+ public static String toJSON(String filename, NavigableSet
blocks)
Review comment:
sure will do, this was also due to backport
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
