[GitHub] [incubator-sdap-nexus] eamonford commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…

[GitBox <gi...@apache.org>]

eamonford commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r441780658



##########
File path: analysis/webservice/algorithms/doms/DomsInitialization.py
##########
@@ -49,6 +49,7 @@ def init(self, config):
         log.info("Cassandra Protocol Version: %s" % (cassVersion))
 
         dc_policy = DCAwareRoundRobinPolicy(cassDatacenter)
+        #dc_policy = WhiteListRoundRobinPolicy([cassHost])

Review comment:
       Could you delete this commented line and the unused import in this file?

##########
File path: data-access/nexustiles/dao/SolrProxy.py
##########
@@ -36,12 +36,18 @@ class SolrProxy(object):
     def __init__(self, config):
         self.solrUrl = config.get("solr", "host")
         self.solrCore = config.get("solr", "core")
+        solr_kargs = {}
+        if config.has_option("solr", "time_out"):
+            solr_kargs["timeout"] = config.get("solr", "time_out")
         self.logger = logging.getLogger('nexus')
 
         with SOLR_CON_LOCK:
             solrcon = getattr(thread_local, 'solrcon', None)
             if solrcon is None:
-                solrcon = pysolr.Solr('http://%s/solr/%s' % (self.solrUrl, self.solrCore))
+                solr_url = 'http://%s/solr/%s' % (self.solrUrl, self.solrCore)
+                self.logger.info("connect to solr, url {}".format(solr_url))
+                self.logger.info("with option(s) = {}".format(solr_kargs))

Review comment:
       Could you combine these two log statements into one? The NEXUS logs are kind of polluted and these two logs are part of the same sentence anyway

##########
File path: analysis/webservice/webapp.py
##########
@@ -156,6 +155,23 @@ def async_callback(self, result):
         if hasattr(result, 'cleanup'):
             result.cleanup()
 
+def inject_args_in_config(args, config):
+    """
+        Takes command argparse arguments and push them in the config
+         with syntax args.<section>_<option>
+    """
+    for _, t_opt in args._options.items():

Review comment:
       you can change this line to `for t_opt in args._options.values():`

##########
File path: analysis/webservice/webapp.py
##########
@@ -156,6 +155,23 @@ def async_callback(self, result):
         if hasattr(result, 'cleanup'):
             result.cleanup()
 
+def inject_args_in_config(args, config):

Review comment:
       Can you write a unit test for this function? There is some parsing logic which should be unit tested

##########
File path: data-access/nexustiles/dao/CassandraProxy.py
##########
@@ -168,6 +168,7 @@ def __init__(self, config):
     def __open(self):
 
         dc_policy = DCAwareRoundRobinPolicy(self.__cass_local_DC)
+        #dc_policy = WhiteListRoundRobinPolicy([self.__cass_url ])

Review comment:
       Could you delete this commented line and the unused import in this file?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org