You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/10/10 08:45:48 UTC
[GitHub] [beam] brachi-wernick opened a new pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
brachi-wernick opened a new pull request #15699:
URL: https://github.com/apache/beam/pull/15699
This PR is a continuous work for https://github.com/apache/beam/pull/15510.
Currently there are 2 Coders for Metadata: default one: `org.apache.beam.sdk.io.fs.MetadataCoder` and enhanced one `org.apache.beam.sdk.io.fs.MetadataCoderV2`, the last can also decode-encode `lastModifiedMillis` and it is done in a new coder in order to support backward compatibility.
This will be hard to maintain, we will need to create a new coder for any new field that will be added to `Metadata`.
So, as suggested in this comment: https://github.com/apache/beam/pull/15510#issuecomment-928390587, I came up with new generic coder : `MetadataDynamicCoder`.
`MetadataDynamicCoder` can decode/encode any new fields added to `Metadata` by sending getter, setter and coder.
For example creating coder for `lastModifiedMillis`:
``` java
new MetadataDynamicCoder()
.withCoderForField(
VarLongCoder.of(),
Metadata::lastModifiedMillis,
Metadata.Builder::setLastModifiedMillis);
```
I chose to get explicit getter/setter to avoid reflection which has bad impact on performance.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-952266859
Adding @pabloem since he reviewed the other PR associated with this Jira, and CC @robertwb
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-991450130
@pabloem - a friendly ping for a 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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-991450130
@pabloem - a friendly ping for a 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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-983926316
> @aaltay I would say to merge it as additional dynamic coder(I mean addition to V1 and V2), we can open another JIRA ticket and PR to delete old coders as @pabloem said. WDYT?
That sounds reasonable to me. We can merge it once @pabloem completes his 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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-13640][BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-1043865717
@chamikaramj @kerrydc - Could you please help to find a reviewer?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-13640][BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-1018055079
@pabloem - Could you please take a look?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] pabloem commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-964449526
This is pretty cool. Thanks @brachi-wernick !
I think it makes sense to create a JIRA issue with target version 3.0.0 to remove the V1 and V2 coders, and rely directly on the latest versions of these coders. WDYT?
I will ask someone who knows coders better to verify that this coder implementation is backwards-compatible for the addition of new fields.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] brachipa commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
brachipa commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-949269580
Please don't, the other PR fixed the ReadableFileCoder, and this one fixes the Mrtadatacoder itself.
To address this suggestion/comment https://github.com/apache/beam/pull/15510#issuecomment-928390587
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-949165230
The other PR seems to be merged. Could we close 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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-982850615
Folks, what are the next steps on this PR?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-13640][BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-1022773961
@pabloem - friendly ping. Could you review or find another reviewer?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] brachi-wernick commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
brachi-wernick commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-939442710
@pabloem @robertwb new PR submitted #15699 to address MetadataCoder to be more extendable for new fields. can 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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] brachi-wernick commented on pull request #15699: [BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
brachi-wernick commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-983040862
@aaltay I would say to merge it as additional dynamic coder(I mean addition to V1 and V2), we can open another JIRA ticket and PR to delete old coders as @pabloem said. WDYT?
This is not critical for now to merge it, but we will need it in the day someone will add more fields to Metadata 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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #15699: [BEAM-13640][BEAM-12883] Add MetadataDynamicCoder to support encode-decode for new fields in Metatdata
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #15699:
URL: https://github.com/apache/beam/pull/15699#issuecomment-1030502672
@pabloem - friendly ping? Perhaps @kerrydc could help with finding another reviewer?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org