Merge lp:~ensoft-opensource/ensoft-sextant/minor_bugfixes into lp:ensoft-sextant

Proposed by Patrick Stevens
Status: Merged
Approved by: ChrisD
Approved revision: 13
Merged at revision: 13
Proposed branch: lp:~ensoft-opensource/ensoft-sextant/minor_bugfixes
Merge into: lp:ensoft-sextant
Prerequisite: lp:~ensoft-opensource/ensoft-sextant/auditing
Diff against target: 180 lines (+33/-19)
5 files modified
setup.py (+4/-0)
src/sextant/__main__.py (+19/-5)
src/sextant/db_api.py (+6/-2)
src/sextant/environment.py (+1/-1)
src/sextant/web/server.py (+3/-11)
To merge this branch: bzr merge lp:~ensoft-opensource/ensoft-sextant/minor_bugfixes
Reviewer Review Type Date Requested Status
Patrick Stevens Approve
Review via email: mp+232219@code.launchpad.net

Commit message

Make Sextant Web a lot less chatty.

Description of the change

Make Sextant Web a lot less chatty.
Variety of other small bugfixes.

To post a comment you must log in.
Revision history for this message
Patrick Stevens (patrickas) :
review: Approve
Revision history for this message
Martin Morrison (isoschiz) wrote :

The prerequisite lp:~ensoft-opensource/ensoft-sextant/auditing has not yet been merged into lp:ensoft-sextant.

Revision history for this message
ChrisD (gingerchris) wrote :

Did we work out why info logging was coming out?
As discussed I thought logging defaulted to the level above info.

Revision history for this message
Patrick Stevens (patrickas) wrote :

I think it's because of Phil's addition:

logging.config.dictConfig({
         "version": 1,
         "handlers": {
             "console": {
                 "class": "logging.StreamHandler",
                 "level": logging.INFO,
                 "stream": "ext://sys.stderr",
             },
         },
         "root": {
             "level": logging.DEBUG,
             "handlers": ["console"],
         },
     })

which appears in sextant.environment or sextant.__main__, depending on
which revision you're looking at.

Patrick

On 26/08/14 15:02, Chris wrote:
> Did we work out why info logging was coming out?
> As discussed I thought logging defaulted to the level above info.

Revision history for this message
ChrisD (gingerchris) wrote :

Ok, I'd remove that "level" line - I doubt we want debug level on by
default.

I'd suggest we do as the @@@ suggests and add something like

        parser.add_argument('-v', '--verbose',
                            action='store_true', help="Show debug")
...
        if args.verbose:
            logging.setLevel(logging.DEBUG)

On 26 August 2014 15:06, Patrick Stevens <email address hidden> wrote:

> I think it's because of Phil's addition:
>
> logging.config.dictConfig({
> "version": 1,
> "handlers": {
> "console": {
> "class": "logging.StreamHandler",
> "level": logging.INFO,
> "stream": "ext://sys.stderr",
> },
> },
> "root": {
> "level": logging.DEBUG,
> "handlers": ["console"],
> },
> })
>
> which appears in sextant.environment or sextant.__main__, depending on
> which revision you're looking at.
>
> Patrick
>
> On 26/08/14 15:02, Chris wrote:
> > Did we work out why info logging was coming out?
> > As discussed I thought logging defaulted to the level above info.
>
>
> --
>
> https://code.launchpad.net/~ensoft-opensource/ensoft-sextant/minor_bugfixes/+merge/232219
> Your team Ensoft Open Source is subscribed to branch
> lp:~ensoft-opensource/ensoft-sextant/auditing.
>

6. By James Harkin <email address hidden>

remove usage: __main__.py changed to SEXTANT

7. By James Harkin <email address hidden>

merge

8. By James Harkin <email address hidden>

merge

Revision history for this message
Patrick Stevens (patrickas) wrote :

By the way, the argparse-parent-parsers thing was why I didn't implement
this.

On 26/08/14 15:39, Chris wrote:
> Ok, I'd remove that "level" line - I doubt we want debug level on by
> default.
>
> I'd suggest we do as the @@@ suggests and add something like
>
> parser.add_argument('-v', '--verbose',
> action='store_true', help="Show debug")
> ...
> if args.verbose:
> logging.setLevel(logging.DEBUG)
>
>
>
>
>
> On 26 August 2014 15:06, Patrick Stevens <email address hidden> wrote:
>
>> I think it's because of Phil's addition:
>>
>> logging.config.dictConfig({
>> "version": 1,
>> "handlers": {
>> "console": {
>> "class": "logging.StreamHandler",
>> "level": logging.INFO,
>> "stream": "ext://sys.stderr",
>> },
>> },
>> "root": {
>> "level": logging.DEBUG,
>> "handlers": ["console"],
>> },
>> })
>>
>> which appears in sextant.environment or sextant.__main__, depending on
>> which revision you're looking at.
>>
>> Patrick
>>
>> On 26/08/14 15:02, Chris wrote:
>>> Did we work out why info logging was coming out?
>>> As discussed I thought logging defaulted to the level above info.
>>
>> --
>>
>> https://code.launchpad.net/~ensoft-opensource/ensoft-sextant/minor_bugfixes/+merge/232219
>> Your team Ensoft Open Source is subscribed to branch
>> lp:~ensoft-opensource/ensoft-sextant/auditing.
>>

9. By Patrick Stevens <email address hidden>

Merge personal minor_bugfixes

10. By Patrick Stevens <email address hidden>

Merge trunk (SSH branch)

11. By Patrick Stevens <email address hidden>

Fix logging level ('warning' -> 'info')

Revision history for this message
Patrick Stevens (patrickas) wrote :

Is it OK for this to be approved without the --verbose or --quiet options? They seem more like feature requests than bugfixes anyway. Currently invoking Sextant from the command line prints out which config file we're using, then if we're in Sextant Web it prints which port we're serving on, while in sextant add-program it prints "Objdump has parsed", "Sending <num> objects to server" and "Sending complete! Exiting".

It also erroneously prints "Exit request sent" on completion of every command when ssh was used (that's because of a bug in ssh, which is patched in Debian but not Ubuntu). That's a future bugfix, I think.

Revision history for this message
ChrisD (gingerchris) :
Revision history for this message
Patrick Stevens (patrickas) wrote :

One comment

Revision history for this message
ChrisD (gingerchris) wrote :

Yes fine to approve without those options, I agree they are outside the scope of this task.

12. By Patrick Stevens <email address hidden>

Handle an error where the user specified a non-existent place to SSH to

13. By Patrick Stevens <email address hidden>

SEXTANT -> sextant in usage message

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'setup.py'
--- setup.py 2014-08-29 13:10:17 +0000
+++ setup.py 2014-09-01 15:05:19 +0000
@@ -18,7 +18,11 @@
18 packages=['sextant', 'sextant.web', 'resources', 'etc'],18 packages=['sextant', 'sextant.web', 'resources', 'etc'],
19 package_dir={'sextant': 'src/sextant', 'resources': 'resources', 'etc': 'etc'},19 package_dir={'sextant': 'src/sextant', 'resources': 'resources', 'etc': 'etc'},
20 scripts=['bin/sextant'],20 scripts=['bin/sextant'],
21 # neo4jrestclient has no python-neo4jrestclient package on Ubuntu 12,
22 # so we remove its dependency here. You *must* install neo4jrestclient
23 # somehow before using Sextant; we recommend pip.
21 install_requires=['twisted'],24 install_requires=['twisted'],
25 #install_requires=['neo4jrestclient', 'twisted'],
22 package_data={'resources': ['sextant/web/*'], 'etc': ['*.conf']},26 package_data={'resources': ['sextant/web/*'], 'etc': ['*.conf']},
23)27)
2428
2529
=== modified file 'src/sextant/__main__.py'
--- src/sextant/__main__.py 2014-08-29 14:18:22 +0000
+++ src/sextant/__main__.py 2014-09-01 15:05:19 +0000
@@ -82,7 +82,7 @@
82 st = ('Program {progname} with {numfuncs} functions '82 st = ('Program {progname} with {numfuncs} functions '
83 'uploaded by {uploader} (id {uploaderid}) on {date}.')83 'uploaded by {uploader} (id {uploaderid}) on {date}.')
84 print(st.format(progname=program.program_name,84 print(st.format(progname=program.program_name,
85 numfuncs=program.number,85 numfuncs=program.number_of_funcs,
86 uploader=program.uploader,86 uploader=program.uploader,
87 uploaderid=program.uploader_id,87 uploaderid=program.uploader_id,
88 date=program.date))88 date=program.date))
@@ -164,7 +164,7 @@
164164
165 """165 """
166166
167 argumentparser = argparse.ArgumentParser(description="Invoke part of the SEXTANT program")167 argumentparser = argparse.ArgumentParser(prog='sextant', usage='sextant', description="Invoke part of the SEXTANT program")
168 subparsers = argumentparser.add_subparsers(title="subcommands")168 subparsers = argumentparser.add_subparsers(title="subcommands")
169169
170 #set what will be defined as a "common function"170 #set what will be defined as a "common function"
@@ -368,8 +368,16 @@
368368
369369
370def _is_localhost(host, port):370def _is_localhost(host, port):
371 """Checks whether a host is an alias to localhost."""371 """
372 return socket.getaddrinfo(host, port)[0][4][0] in ('127.0.0.1', '::1')372 Checks whether a host is an alias to localhost.
373
374 Raises socket.gaierror if the host was not found.
375
376 """
377
378 addr = socket.getaddrinfo(host, port)[0][4][0]
379
380 return addr in ('127.0.0.1', '::1')
373381
374382
375def main():383def main():
@@ -380,7 +388,13 @@
380388
381 remotehost, remoteport = _get_host_and_port(args.remote_neo4j)389 remotehost, remoteport = _get_host_and_port(args.remote_neo4j)
382390
383 if _is_localhost(remotehost, remoteport):391 try:
392 is_loc = _is_localhost(remotehost, remoteport)
393 except socket.gaierror:
394 logging.error('Server {} not found.'.format(remotehost))
395 return
396
397 if is_loc:
384 # we are attempting to connect to localhost anyway, so we won't398 # we are attempting to connect to localhost anyway, so we won't
385 # bother to SSH to it.399 # bother to SSH to it.
386 # There may be some ways the user can trick us into trying to SSH400 # There may be some ways the user can trick us into trying to SSH
387401
=== modified file 'src/sextant/db_api.py'
--- src/sextant/db_api.py 2014-08-22 14:23:32 +0000
+++ src/sextant/db_api.py 2014-09-01 15:05:19 +0000
@@ -374,7 +374,8 @@
374374
375 ProgramWithMetadata = namedtuple('ProgramWithMetadata',375 ProgramWithMetadata = namedtuple('ProgramWithMetadata',
376 ['uploader', 'uploader_id',376 ['uploader', 'uploader_id',
377 'program_name', 'date', 'number'])377 'program_name', 'date',
378 'number_of_funcs'])
378379
379 def __init__(self, url):380 def __init__(self, url):
380 self.url = url381 self.url = url
@@ -459,10 +460,13 @@
459 def programs_with_metadata(self):460 def programs_with_metadata(self):
460 """461 """
461 Returns a set of namedtuples which represent the current database.462 Returns a set of namedtuples which represent the current database.
463
462 The namedtuples have .uploader, .uploader_id, .program_name, .date,464 The namedtuples have .uploader, .uploader_id, .program_name, .date,
463 .number (of functions)465 .number_of_funcs.
464 :return: set of namedtuples466 :return: set of namedtuples
467
465 """468 """
469
466 q = ("MATCH (base) WHERE base.type = 'program' "470 q = ("MATCH (base) WHERE base.type = 'program' "
467 "MATCH (base)-[:subject]->(n)"471 "MATCH (base)-[:subject]->(n)"
468 "RETURN base.uploader, base.uploader_id, base.name, base.date, count(n)")472 "RETURN base.uploader, base.uploader_id, base.name, base.date, count(n)")
469473
=== modified file 'src/sextant/environment.py'
--- src/sextant/environment.py 2014-08-29 13:19:53 +0000
+++ src/sextant/environment.py 2014-09-01 15:05:19 +0000
@@ -75,7 +75,7 @@
75 "handlers": {75 "handlers": {
76 "console": {76 "console": {
77 "class": "logging.StreamHandler",77 "class": "logging.StreamHandler",
78 "level": logging.WARNING,78 "level": logging.INFO,
79 "stream": "ext://sys.stderr",79 "stream": "ext://sys.stderr",
80 },80 },
81 },81 },
8282
=== modified file 'src/sextant/web/server.py'
--- src/sextant/web/server.py 2014-08-26 11:04:04 +0000
+++ src/sextant/web/server.py 2014-09-01 15:05:19 +0000
@@ -84,7 +84,6 @@
84 request.finish()84 request.finish()
85 defer.returnValue(None)85 defer.returnValue(None)
8686
87 logging.info('enter')
88 name = request.args["program_name"][0]87 name = request.args["program_name"][0]
8988
90 try:89 try:
@@ -106,16 +105,14 @@
106 defer.returnValue(None)105 defer.returnValue(None)
107 neo4jconnection = None # to silence the "referenced before assignment" warnings later106 neo4jconnection = None # to silence the "referenced before assignment" warnings later
108107
109 logging.info('created')
110 exists = yield deferToThread(self.check_program_exists, neo4jconnection, name)108 exists = yield deferToThread(self.check_program_exists, neo4jconnection, name)
111 if not exists:109 if not exists:
112 request.setResponseCode(404)110 request.setResponseCode(404)
113 logging.info('returning nonexistent')111 logging.debug('returning nonexistent')
114 request.write("Name %s not found." % (escape(name)))112 request.write("Name %s not found." % (escape(name)))
115 request.finish()113 request.finish()
116 defer.returnValue(None)114 defer.returnValue(None)
117115
118 logging.info('done created')
119 allowed_queries = ("whole_program", "functions_calling", "functions_called_by", "call_paths", "shortest_path")116 allowed_queries = ("whole_program", "functions_calling", "functions_called_by", "call_paths", "shortest_path")
120117
121 if "query" not in request.args:118 if "query" not in request.args:
@@ -198,8 +195,7 @@
198 request.finish()195 request.finish()
199 defer.returnValue(None)196 defer.returnValue(None)
200197
201 logging.info('getting plot')198 logging.debug('getting plot')
202 logging.info(program)
203 if not program.functions: # we got an empty program back: the program is in the Sextant but has no functions199 if not program.functions: # we got an empty program back: the program is in the Sextant but has no functions
204 request.setResponseCode(204)200 request.setResponseCode(204)
205 request.finish()201 request.finish()
@@ -208,7 +204,7 @@
208 remove_self_calls=False)204 remove_self_calls=False)
209 request.setHeader("content-type", "image/svg+xml")205 request.setHeader("content-type", "image/svg+xml")
210206
211 logging.info('SVG: return')207 logging.debug('SVG: return')
212 request.write(output)208 request.write(output)
213 request.finish()209 request.finish()
214210
@@ -242,8 +238,6 @@
242238
243 query = request.args['query'][0]239 query = request.args['query'][0]
244240
245 logging.info('Properties: about to get_connection')
246
247 try:241 try:
248 neo4j_connection = yield deferToThread(self._get_connection)242 neo4j_connection = yield deferToThread(self._get_connection)
249 except Exception:243 except Exception:
@@ -253,8 +247,6 @@
253 defer.returnValue(None)247 defer.returnValue(None)
254 neo4j_connection = None # just to silence the "referenced before assignment" warnings248 neo4j_connection = None # just to silence the "referenced before assignment" warnings
255249
256 logging.info('got connection')
257
258 if query == 'programs':250 if query == 'programs':
259 request.setHeader("content-type", "application/json")251 request.setHeader("content-type", "application/json")
260 prognames = yield deferToThread(self._get_program_names, neo4j_connection)252 prognames = yield deferToThread(self._get_program_names, neo4j_connection)

Subscribers

People subscribed via source and target branches