Issue #6: Support for webservice version in cli and flask api endpoint #41
Conversation
There was a problem hiding this comment.
Thanks for taking care of this! Looks great already :) Makes a lot of sense too, to not have --version yet.
Please have a look at the travis build if still failing after you implemented the review changes.
Also, can you add a small test to test_cli (in CliFlaskTestCase) to ensure code coverage?
Another thing I thought about was changing the command name to webservice since it's not supported when people use it with the 'none' service (which could still be considered a service, albeit fake). This also brings me to the question how to handle the command with the none service. With the current way, Server.run() would return an error. Either we tell the user that the service/webservice command is only to be used with the flask service; or we add support for the service command when the 'none' service is used. I tend towards having a webservice command; and giving an error at cli module level if anyone tries to invoke it while the none service is configured to be used.
| @@ -63,5 +63,9 @@ def create_app(data_dir=None, config=None): | |||
| EntryResource, | |||
| "{}/<period_name>/<table_name>/<eid>".format(PERIODS_TAIL), | |||
| resource_class_args=(server,)) | |||
| api.add_resource( | |||
| VersionResource, | |||
| '/version' | |||
There was a problem hiding this comment.
Can you introduce a constant VERSION_TAIL in financeager/__init__.py for the /version part; and also substitute the use in the httprequests module?
| @@ -120,3 +122,9 @@ class CopyResource(LogResource): | |||
| def post(self): | |||
| args = copy_parser.parse_args() | |||
| return self.run_safely("copy", error_code=404, **args) | |||
|
|
|||
|
|
|||
| class VersionResource(Resource): | |||
There was a problem hiding this comment.
Can you derive the class from LogResource. This automatically logs the incoming request which can be helpful for debugging.
| class VersionResource(Resource): | ||
| def get(self): | ||
| val = {'version': __version__} | ||
| return val |
There was a problem hiding this comment.
I think you can shorten this to return {...}.
Also please add the missing EOF (it's one thing the travis build complained about).
|
Closed in pylipp/financeager-flask@7164e6b |
This creates a new option to return the version of the webservice via a
servicecommand.Note that I dropped the use of
--versionsince the service endpoint only returns one thing at this point.