copyTransient is not global #375
Conversation
|
I understand that you want to have a global setting for copyTransient inside a Kryo instance. But I'm not quite sure about removing the ability to set it on per FieldSerializer basis. I think it should still be possible to change it for a concrete instance of FieldSerializer. Instead, in the constructor of FieldSerializer you could take the initial value for copyTransient from the Kryo instance. Does it make sense to you? |
|
Yes. @romix That makes sense. I will make those changes and will update the pull request so as to not break compatibility. |
|
@romix Updated the PR as per your review comments. |
|
I will add a test case shortly |
|
@romix Can you please review this PR? |
| * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ | ||
|
|
||
| * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ | ||
There was a problem hiding this comment.
What is this file header touched at all?
There was a problem hiding this comment.
Line no: 18, 19 and EOF was missing CRLF. IntelliJ inserted the line feed automatically.
|
Overall, the PR looks good to me. I've commented on a few indentation issues. May be there are others. Please have a look and fix them. @NathanSweet @magro I think with those small formatting issues fixed, it is OK to merge. Any objections? |
|
Sorry to bother again. We are looking forward for this feature in Apache Hive. It will be great if we can get this in. |
|
LGTM. @NathanSweet? |
Allow to set copyTransient globally for all FieldSerializer instances
Fix for #370
There were couple of approaches discussed in mailing list. Making it a config in kryo object seems to be easier.