Re: [PR] feat(java): extract public Fury methods to BaseFury [incubator-fury]
chaokunyang merged PR #1467: URL: https://github.com/apache/incubator-fury/pull/1467 -- 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-unsubscr...@fury.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org For additional commands, e-mail: commits-h...@fury.apache.org
Re: [PR] feat(java): extract public Fury methods to BaseFury [incubator-fury]
chaokunyang commented on code in PR #1467: URL: https://github.com/apache/incubator-fury/pull/1467#discussion_r1553444979 ## java/fury-core/src/main/java/org/apache/fury/BaseFury.java: ## @@ -64,9 +71,16 @@ public interface BaseFury { */ void registerSerializer(Class type, Class serializerClass); + void registerSerializer(Class type, Serializer serializer); Review Comment: I see, thanks. This is reasonable to add to BaseFury -- 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-unsubscr...@fury.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org For additional commands, e-mail: commits-h...@fury.apache.org
Re: [PR] feat(java): extract public Fury methods to BaseFury [incubator-fury]
Munoon commented on code in PR #1467: URL: https://github.com/apache/incubator-fury/pull/1467#discussion_r1553192936 ## java/fury-core/src/main/java/org/apache/fury/BaseFury.java: ## @@ -64,9 +71,16 @@ public interface BaseFury { */ void registerSerializer(Class type, Class serializerClass); + void registerSerializer(Class type, Serializer serializer); Review Comment: There might be a singleton serializer. I've personally create some of them in my project. In this case they will be applied to each existing and each new Fury instances in `ThreadLocalFury` and `ThreadPoolFury`. Maybe this is something that worth mentioning in the documentation. However, if you didn't like it, we can remove this method and make user register singleton serializers using `execute` method. In this case, there is more chances that user will know what he is doing. -- 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-unsubscr...@fury.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org For additional commands, e-mail: commits-h...@fury.apache.org
Re: [PR] feat(java): extract public Fury methods to BaseFury [incubator-fury]
chaokunyang commented on code in PR #1467: URL: https://github.com/apache/incubator-fury/pull/1467#discussion_r1552605327 ## java/fury-core/src/main/java/org/apache/fury/BaseFury.java: ## @@ -55,6 +59,9 @@ public interface BaseFury { */ void register(Class cls, Short id, boolean createSerializer); + /** register class with given type tag which will be used for cross-language serialization. */ + void register(Class cls, String typeTag); Review Comment: This API is used for clang serialization, we removed it in our new xlang serialization spec in #1413 . Could we remove this API? We can add new API when new protocol are implemented -- 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-unsubscr...@fury.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org For additional commands, e-mail: commits-h...@fury.apache.org
Re: [PR] feat(java): extract public Fury methods to BaseFury [incubator-fury]
chaokunyang commented on code in PR #1467: URL: https://github.com/apache/incubator-fury/pull/1467#discussion_r1552603974 ## java/fury-core/src/main/java/org/apache/fury/BaseFury.java: ## @@ -64,9 +71,16 @@ public interface BaseFury { */ void registerSerializer(Class type, Class serializerClass); + void registerSerializer(Class type, Serializer serializer); Review Comment: The serializer is bind to a Fury instance, we may not be able to create a serializer object for registration unless we change the serializer interfere. Could we change this to register(Class type, Class serializer class)? -- 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-unsubscr...@fury.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org For additional commands, e-mail: commits-h...@fury.apache.org
[PR] feat(java): extract public Fury methods to BaseFury [incubator-fury]
Munoon opened a new pull request, #1467: URL: https://github.com/apache/incubator-fury/pull/1467 Extract public Fury method to the `BaseFury` interface and implement them in `ThreadPoolFury` and `ThreadLocalFury`, so end users can avoid using `execute()` and other utility methods. -- 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-unsubscr...@fury.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org For additional commands, e-mail: commits-h...@fury.apache.org