Add new REST-based Webapi and interactive webapi Docs with Swagger-UI #276

bendikro · 2020-04-01T15:20:03Z

The docstring_parser library uses type annotation syntax not supported in the python version on travis.

cas--

Swagger is a good choice, I would prefer not storing the source code in codebase. Can we install with npm and copy what is needed in setup/install?

Is webapidoc is bit long-winded? Would dropping the web prefix apidoc make it too generic and confused with deluge api? wapidoc, restdoc? Not sure here if there is much other option, any thoughts?

Also the endpoints seem very long too '/webapidocs/web/deregister_event_listener', at least drop the 'webapidocs' part? Wonder if we customize, such as:

/torrent/files
/torrent/info
/events/register
/events/deregister

Which leads to another question, does the api need a version too?

setup.py

deluge/httpdownloader.py

cas-- · 2020-05-10T09:40:38Z

The openapi docs are served by the app but perhaps we also want them in docs with https://github.com/sphinx-contrib/openapi

bendikro · 2020-05-10T20:07:10Z

Swagger is a good choice, I would prefer not storing the source code in codebase. Can we install with npm and copy what is needed in setup/install?

Certainly. I just included the source for simplicity. How (where) would you suggest doing this?

Is webapidoc is bit long-winded? Would dropping the web prefix apidoc make it too generic and confused with deluge api? wapidoc, restdoc? Not sure here if there is much other option, any thoughts?

Can change to api to make it shorter.

Also the endpoints seem very long too '/webapidocs/web/deregister_event_listener', at least drop the 'webapidocs' part? Wonder if we customize, such as:
/torrent/files
/torrent/info
/events/register
/events/deregister

webapidocs is the base prefix, so after changing that to api, the next part, web reflects the path of the function in the source. This matches the values given in the get_methods function for the old json api endpoint.

curl -X GET "http://0.0.0.0:8112/api/web/get_methods?id=1&pretty=pretty" -H "accept: application/json"
{
  "result": [
    "web.add_host",
    "web.add_torrents",
    "web.connect",
    "web.connected",
....
...

cas-- · 2020-05-12T09:12:23Z

deluge/plugins/WebUi/deluge_webui/core.py

+DEFAULT_PREFS = {
+    'enabled': False,
+    'ssl': False,
+    'webapidoc_enabled': False,


Do we need a config to enable/disable docs?

If so should we enable it by default? Users will never know about it and like me annoyed when they need to set a config ;)

And rename this setting to just docs?

I'm not sure. You think it should always be available? You'll have to log in to do anything of importance anyways, so maybe we can drop the setting.

cas-- · 2020-05-12T09:13:50Z

deluge/plugins/WebUi/deluge_webui/core.py

@@ -75,6 +86,7 @@ def start_server(self):

        self.server.port = self.config['port']
        self.server.https = self.config['ssl']
+        self.server.setup_webapidoc(enabled=self.config['webapidoc_enabled'])


I don't this setup should be part of the plugin code, the server itself should check config and do the setup

cas-- · 2020-05-12T09:20:01Z

deluge/ui/web/server.py

@@ -748,6 +831,14 @@ def start_normal(self):
        ip = self.socket.getHost().host
        ip = '[%s]' % ip if is_ipv6(ip) else ip
        log.info('Serving at http://%s:%s%s', ip, self.port, self.base)
+        if self.webapidoc_enabled:


I'm not sure what value this log entry adds

It shows where the docs are served. Isn't that useful in itself?

cas-- · 2020-05-12T09:20:06Z

deluge/ui/web/server.py

@@ -765,6 +856,14 @@ def start_ssl(self):
        ip = self.socket.getHost().host
        ip = '[%s]' % ip if is_ipv6(ip) else ip
        log.info('Serving at https://%s:%s%s', ip, self.port, self.base)
+        if self.webapidoc_enabled:


I'm not sure what value this log entry adds

cas-- · 2020-05-12T09:20:26Z

deluge/ui/web/server.py

@@ -740,6 +819,10 @@ def start(self):

        component.get('Web').enable()

+        # TODO: Check if correct
+        # Plugins must be enabled here to register exported json functions
+        self.plugins.enable_plugins()


Were they not exported before?

I think the problem I had was that the plugins list in the config was empty unless enabled here.

cas-- · 2020-05-12T09:21:36Z

deluge/ui/web/server.py

-        self.plugins.disable_plugins()
+        # TODO: Check if this is correct
+        # Disabling plugins removes them from enabled_plugins list in web.conf???
+        # self.plugins.disable_plugins()


Interesting, could we put these plugin issues in a separate commit?

cas-- · 2020-05-12T09:23:10Z

deluge/ui/web/server.py

@@ -724,6 +798,11 @@ def win_handler(ctrl_type):

            SetConsoleCtrlHandler(win_handler)

+    def setup_webapidoc(self, enabled=True):


Duplicate method naming, what is the purpose of this one?

cas-- · 2020-05-12T09:23:35Z

deluge/ui/web/server.py

@@ -536,6 +585,28 @@ def __init__(self):
            theme = CONFIG_DEFAULTS.get('theme')
        self.__stylesheets.insert(1, 'themes/css/xtheme-%s.css' % theme)

+    def setup_webapidoc(self, enabled=True):


I feel that enabling the docs should be read from config not passed as arg, thoughts?

cas-- · 2020-05-12T09:25:38Z

deluge/ui/web/server.py

+
+            self.putChild(self.webapidoc_path.encode(), self.webapidoc_resource)
+            self.js.add_script_folder(path, rpath('js', path))
+        else:


Is this to handle users switching the config from enable to disable?

Yes, it is. Would be easier to just have it always enabled

cas-- · 2020-05-12T09:45:51Z

setup.py

@@ -536,6 +536,8 @@ def run(self):

 setup_requires = ['setuptools', 'wheel']
 install_requires = [
+    "apispec; python_version >= '3'",


Just thought that I'm not sure we need to pin this to Python 3? Only apispec v3.0.0 is Python 3 only, are you using v3 specific methods: https://apispec.readthedocs.io/en/latest/changelog.html#id5

Honestly, I'm not sure as I haven't tested on python 2.

…r-UI The new webapi served on /api creates separate endpoints for each exported python function, e.g. api/web/login Webapi functions are exported with webapi = WebapiNamespace("Webapi") @webapi.post def exported_post_func(self, arg): ... @webapi.get def exported_get_func(self): ... The interactive webapi documenation is generated with swagger-ui and served on http://0.0.0.0:8112/webapidoc The function doc of the exported functions is parsed with GoogleStyleDocPlugin that relies on 'docstring_parser' which only supported on python >= 3.6. With python < 3.6, these doc strings are not included in the api docs.

There might be a bug causing issues with plugin functions not being registered with the json api. This should eb investigated

… Swagger-UI

bendikro force-pushed the Issue/develop/3308/new-webapi-swagger-ui branch 2 times, most recently from e829120 to e25f7e6 Compare April 12, 2020 19:08

cas-- reviewed May 9, 2020

View reviewed changes

setup.py Outdated Show resolved Hide resolved

deluge/httpdownloader.py Outdated Show resolved Hide resolved

bendikro force-pushed the Issue/develop/3308/new-webapi-swagger-ui branch 3 times, most recently from 4902225 to 5dd38ef Compare May 11, 2020 18:50

cas-- reviewed May 12, 2020

View reviewed changes

bendikro force-pushed the Issue/develop/3308/new-webapi-swagger-ui branch from f2b4d00 to 81ae652 Compare September 15, 2020 12:14

bendikro and others added 4 commits September 15, 2020 14:34

Provide better logging on errors in DelugeTransferProtocol

be21ddf

Add script to download Swagger UI with npm

0343fbb

Some issue with registering the correct functions for the api

7a53887

There might be a bug causing issues with plugin functions not being registered with the json api. This should eb investigated

bendikro force-pushed the Issue/develop/3308/new-webapi-swagger-ui branch from 81ae652 to 7a53887 Compare September 15, 2020 12:46

fixup! [Web] Add new webapi endpoint and interactive webapi docs with…

b1e47b1

… Swagger-UI

deluge-torrent deleted a comment from Erarshad Sep 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new REST-based Webapi and interactive webapi Docs with Swagger-UI #276

Add new REST-based Webapi and interactive webapi Docs with Swagger-UI #276

bendikro commented Apr 1, 2020 •

edited

Loading

cas-- left a comment

cas-- commented May 10, 2020

bendikro commented May 10, 2020

cas-- May 12, 2020

bendikro May 12, 2020

cas-- May 12, 2020

cas-- May 12, 2020

bendikro May 12, 2020

cas-- May 12, 2020

cas-- May 12, 2020

bendikro May 12, 2020

cas-- May 12, 2020

cas-- May 12, 2020

cas-- May 12, 2020

cas-- May 12, 2020

bendikro May 12, 2020

cas-- May 12, 2020 •

edited

Loading

bendikro May 12, 2020

		@@ -724,6 +798,11 @@ def win_handler(ctrl_type):

		SetConsoleCtrlHandler(win_handler)

		def setup_webapidoc(self, enabled=True):

Add new REST-based Webapi and interactive webapi Docs with Swagger-UI #276

Are you sure you want to change the base?

Add new REST-based Webapi and interactive webapi Docs with Swagger-UI #276

Conversation

bendikro commented Apr 1, 2020 • edited Loading

cas-- left a comment

Choose a reason for hiding this comment

cas-- commented May 10, 2020

bendikro commented May 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cas-- May 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bendikro commented Apr 1, 2020 •

edited

Loading

cas-- May 12, 2020 •

edited

Loading