drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #868: DRILL-5547:Linking config options with system optio...
Date Tue, 11 Jul 2017 23:54:48 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message