Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/868#discussion_r126825532
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
    @@ -310,8 +310,10 @@
       /**
        * Limits the maximum level of parallelization to this factor time the 
number of Drillbits
        */
    +  String CPU_LOAD_AVERAGE_KEY = "planner.cpu_load_average";
    +  DoubleValidator CPU_LOAD_AVERAGE = new 
DoubleValidator(CPU_LOAD_AVERAGE_KEY,0.70);
    --- End diff --
    
    Just a bit confused here. The defaults are moving into the 
drill-module.conf file, which is good.
    
    But, we leave hard-coded values in ExecConstants?
    
    The result will be that future developers will continue to set the defaults 
in code, as ever, and not realize that they should set the values in the conf 
file. What happens if the conf file has one value, the hard-coded defaults 
another? Quite the riddle...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to