[ 
https://issues.apache.org/jira/browse/BEAM-12767?focusedWorklogId=645442&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-645442
 ]

ASF GitHub Bot logged work on BEAM-12767:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 01/Sep/21 18:02
            Start Date: 01/Sep/21 18:02
    Worklog Time Spent: 10m 
      Work Description: lukecwik commented on pull request #15338:
URL: https://github.com/apache/beam/pull/15338#issuecomment-910524439


   > I think the patch is actually really simple, I can get another PR going 
right now, or we can revert and re-apply with it fixed. I'm fine with either.
   > 
   > ```
   > diff --git 
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
 
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
   > index 030c56ba59..e20a8641b6 100644
   > --- 
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
   > +++ 
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
   > @@ -44,6 +44,7 @@ import 
com.fasterxml.jackson.databind.node.TreeTraversingParser;
   >  import com.fasterxml.jackson.databind.ser.DefaultSerializerProvider;
   >  import com.fasterxml.jackson.databind.type.TypeBindings;
   >  import com.fasterxml.jackson.databind.util.SimpleBeanPropertyDefinition;
   > +import com.fasterxml.jackson.databind.util.TokenBuffer;
   >  import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
   >  import java.beans.BeanInfo;
   >  import java.beans.IntrospectionException;
   > @@ -499,7 +500,8 @@ public class PipelineOptionsFactory {
   >  
   >    private static final DefaultDeserializationContext 
DESERIALIZATION_CONTEXT =
   >        new 
DefaultDeserializationContext.Impl(MAPPER.getDeserializationContext().getFactory())
   > -          .createInstance(MAPPER.getDeserializationConfig(), null, null);
   > +          .createInstance(
   > +              MAPPER.getDeserializationConfig(), new TokenBuffer(MAPPER, 
false).asParser(), null);
   > ```
   
   You can get both PRs out (forward fix and rollback) and run the additional 
failing tests in the forward fix.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 645442)
    Time Spent: 15h 10m  (was: 15h)

> Improve UX of PipelineOptions parsing
> -------------------------------------
>
>                 Key: BEAM-12767
>                 URL: https://issues.apache.org/jira/browse/BEAM-12767
>             Project: Beam
>          Issue Type: Improvement
>          Components: sdk-java-core
>            Reporter: Steve Niemitz
>            Assignee: Steve Niemitz
>            Priority: P2
>             Fix For: 2.34.0
>
>          Time Spent: 15h 10m
>  Remaining Estimate: 0h
>
> Attempting to parse complex types in PipelineOptions can have a suboptimal 
> UX.  For example, if the option type being parsed is an Instant, simply 
> passing in:
> {{--myInstant=2021-07-17 }}{{will fail.  Instead the user must pass in 
> --myInstant="2021-07-17"}}{{, however, this is complicated by the fact that 
> the quotes will be stripped from the arguments in many cases, requiring a 
> user to actually pass in --myInstant='"2021-07-17"'.  }}
> This can be improved by attempting to parse the input twice, once as-is, and 
> then trying again by automatically wrapping it in quotes.
> Additionally, it's impossible to use a custom JsonDeserializer on a pipeline 
> option property.  We should allow using @JsonDeserialize on the getters to 
> supply a custom one.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to