falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ajay Yadava" <ajayn...@gmail.com>
Subject Re: Review Request 36758: Pagination API breaks backward compatibility.
Date Fri, 24 Jul 2015 11:18:56 GMT


> On July 24, 2015, 7:34 a.m., Pallavi Rao wrote:
> > src/conf/runtime.properties, line 38
> > <https://reviews.apache.org/r/36758/diff/1/?file=1020521#file1020521line38>
> >
> >     Why do we need 2 props here? Shouldn't just one do? If user does not supply
numResults, the value it should default to.
> >     
> >     What is the purpose of introducing an upper limit that will override numResults?
Is there a system limitation that requires us to specify this? If not, I would prefer to keep
it simple and just have one property.

The second property is not introduced as part of this JIRA. The thinking behind introducing
that was to avoid OOM when someone accidentally gives a large numResults. We can discuss whether
to have it or not in a separate JIRA.


> On July 24, 2015, 7:34 a.m., Pallavi Rao wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconCLI.java, line 249
> > <https://reviews.apache.org/r/36758/diff/1/?file=1020514#file1020514line249>
> >
> >     Generally, not a good practice to default an integer to null.. Will require
additional null checks and may cause NPEs otherwise.

Completely agree. But in this case we are just passing null to queryparams and it is absolutely
safe operation and falcon doesn't need to worry about it. Other option was to introduce a
magic value like -1/0 etc. which I didn't want to take.


- Ajay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36758/#review92877
-----------------------------------------------------------


On July 24, 2015, 12:46 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36758/
> -----------------------------------------------------------
> 
> (Updated July 24, 2015, 12:46 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1324
>     https://issues.apache.org/jira/browse/FALCON-1324
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Currently pagination API breaks backward compatibility as it by default returns 10 results.
For example, earlier
> falcon entity -type feed -list 
>  used to return all the feeds but now the same command will return only 10 results. 
> 
> Proposed solution:
> We should make the default number of results configurable in the properties file and
put it to a high value by default so that behavior is backwards compatible.
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/cli/FalconCLI.java e393f82 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java 555bdb7 
>   common/src/main/resources/runtime.properties 3b32463 
>   prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java a3801e9 
>   prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java 310e73b

>   prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java ac3e5db

>   prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java
eb281d3 
>   src/conf/runtime.properties 58dee3d 
>   webapp/src/main/java/org/apache/falcon/resource/InstanceManager.java 9c5538b 
>   webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java ed6f44e

> 
> Diff: https://reviews.apache.org/r/36758/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message