You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sdap.apache.org by GitBox <gi...@apache.org> on 2020/06/17 00:20:00 UTC
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl opened a new pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
tloubrieu-jpl opened a new pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101
…s configuration
As described in jira ticket SDAP-249
----------------------------------------------------------------
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
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
tloubrieu-jpl commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r443932972
##########
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:
Done
----------------------------------------------------------------
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
[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…
Posted by 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
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl merged pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
tloubrieu-jpl merged pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101
----------------------------------------------------------------
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
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
tloubrieu-jpl commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r443932994
##########
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:
Done
----------------------------------------------------------------
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
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
tloubrieu-jpl commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r443915590
##########
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:
ok, I did that.
----------------------------------------------------------------
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
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
tloubrieu-jpl commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r451910342
##########
File path: analysis/webservice/webapp.py
##########
@@ -25,10 +25,16 @@
import pkg_resources
import tornado.web
from tornado.options import define, options, parse_command_line
-
from webservice import NexusHandler
from webservice.webmodel import NexusRequestObject, NexusProcessingException
+logging.basicConfig(
Review comment:
Ok thanks Franck, I moved back this section in __main__
The reason why I moved it initially is that I wanted to use the log outside the __main__. I created a local one instead.
----------------------------------------------------------------
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
[GitHub] [incubator-sdap-nexus] fgreg commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
fgreg commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r444360805
##########
File path: analysis/webservice/webapp.py
##########
@@ -25,10 +25,16 @@
import pkg_resources
import tornado.web
from tornado.options import define, options, parse_command_line
-
from webservice import NexusHandler
from webservice.webmodel import NexusRequestObject, NexusProcessingException
+logging.basicConfig(
Review comment:
Care should be taken when configuring `logging` in globally like this. Now, whenever this module is imported, logging will be configured (unless it was already configured by some other module which also does this). Generally, it is better to configure logging in `__main__`, is there a reason you wanted to move it?
----------------------------------------------------------------
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
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
tloubrieu-jpl commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r443916050
##########
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:
Ok, done
----------------------------------------------------------------
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
[GitHub] [incubator-sdap-nexus] tloubrieu-jpl commented on a change in pull request #101: SDAP-249 : add solr time out client argument to webapp, use it in the data acces…
Posted by GitBox <gi...@apache.org>.
tloubrieu-jpl commented on a change in pull request #101:
URL: https://github.com/apache/incubator-sdap-nexus/pull/101#discussion_r443931960
##########
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:
I wrote some unit test. I did some with mock before with java, I made my first mock in python.
I can not say I had more fun with it 😄
----------------------------------------------------------------
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