Add origin-url alternative to some endpoints that take an origin-id.
ClosedPublic
Actions

Authored by vlorentz on Jun 28 2019, 10:36 PM.

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Commits

rDWAPPSe148b03ef15d: Add origin-url alternative to some endpoints that take an origin-id.

Summary

'revision log' endpoints with complex lookups are excluded
from this commit, as it's impossible to get a non-ambiguous URL
for them without excessive hacks.

Test Plan

Install swh-storage with arc patch D1665 and run pytest.

Diff Detail

Repository

rDWAPPS Web applications

Branch

origin-url-endpoints

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 6622
Build 9245: tox-on-jenkins	Jenkins
Build 9244: arc lint + arc unit

Event Timeline

vlorentz created this revision.Jun 28 2019, 10:36 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 28 2019, 10:36 PM

vlorentz added a child revision: D1667: Deprecate endpoints using an origin-id..Jun 28 2019, 10:37 PM

vlorentz edited the test plan for this revision. (Show Details)Jun 28 2019, 10:38 PM

vlorentz added inline comments.Jun 28 2019, 10:56 PM

swh/web/api/views/origin.py
139	Note the `/get/`, which is needed to disambiguate the URL. Not very pretty, but that's the only solution I got.

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/561/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/561/console

Harbormaster failed remote builds in B6580: Diff 5573!Jun 28 2019, 10:59 PM

vlorentz added a parent revision: D1665: Make all endpoints support origin urls instead of ids..Jul 1 2019, 11:54 AM

The disambiguation for the origin api endpoint can be resolved by applying the diff in my inline comment.

I will check if the same king of trick can be used for the revision endpoints taking an origin in url arguments.

swh/web/api/views/origin.py

139

Indeed, the order of url declarations in Django matters for disambiguation.

I managed to resolve the issue through the following diff

diff --git a/swh/web/api/urls.py b/swh/web/api/urls.py
index afbcaf44..3c3b99cd 100644
--- a/swh/web/api/urls.py
+++ b/swh/web/api/urls.py
@@ -6,8 +6,8 @@
 import swh.web.api.views.content # noqa
 import swh.web.api.views.directory # noqa
 import swh.web.api.views.identifiers # noqa
-import swh.web.api.views.origin # noqa
 import swh.web.api.views.origin_save # noqa
+import swh.web.api.views.origin # noqa
 import swh.web.api.views.person # noqa
 import swh.web.api.views.release # noqa
 import swh.web.api.views.revision # noqa
diff --git a/swh/web/api/views/origin.py b/swh/web/api/views/origin.py
index 0c70e485..cb54f0ca 100644
--- a/swh/web/api/views/origin.py
+++ b/swh/web/api/views/origin.py
@@ -129,103 +129,6 @@ def api_origins(request):
     return response
 
 
-@api_route(r'/origin/(?P<origin_type>[a-z]+)/url/(?P<origin_url>.+)/',
-           'api-1-origin')
-@api_route(r'/origin/(?P<origin_url>.+)/get/', 'api-1-origin')
-@api_route(r'/origin/(?P<origin_id>[0-9]+)/', 'api-1-origin')
-@api_doc('/origin/')
-@format_docstring(return_origin=DOC_RETURN_ORIGIN)
-def api_origin(request, origin_id=None, origin_type=None, origin_url=None):
-    """
-    .. http:get:: /api/1/origin/(origin_url)/get/
-
-        Get information about a software origin.
-
-        :param string origin_url: the origin url
-
-        {return_origin}
-
-        {common_headers}
-
-        **Allowed HTTP Methods:** :http:method:`get`, :http:method:`head`,
-            :http:method:`options`
-
-        :statuscode 200: no error
-        :statuscode 404: requested origin can not be found in the archive
-
-        **Example:**
-
-        .. parsed-literal::
-
-            :swh_web_api:`origin/git/url/https://github.com/python/cpython/`
-
-    .. http:get:: /api/1/origin/(origin_id)/
-
-        Get information about a software origin.
-
-        :param int origin_id: a software origin identifier
-
-        {return_origin}
-
-        {common_headers}
-
-        **Allowed HTTP Methods:** :http:method:`get`, :http:method:`head`,
-            :http:method:`options`
-
-        :statuscode 200: no error
-        :statuscode 404: requested origin can not be found in the archive
-
-        **Example:**
-
-        .. parsed-literal::
-
-            :swh_web_api:`origin/1/`
-
-    .. http:get:: /api/1/origin/(origin_type)/url/(origin_url)/
-
-        Get information about a software origin.
-
-        .. warning::
-
-            This endpoint is deprecated. You should use
-            :http:get:`/api/1/origin/(origin_url)/get/` instead.
-
-        :param string origin_type: the origin type (possible values are
-            ``git``, ``svn``, ``hg``, ``deb``, ``pypi``, ``npm``, ``ftp`` or
-            ``deposit``)
-        :param string origin_url: the origin url
-
-        {return_origin}
-
-        {common_headers}
-
-        **Allowed HTTP Methods:** :http:method:`get`, :http:method:`head`,
-            :http:method:`options`
-
-        :statuscode 200: no error
-        :statuscode 404: requested origin can not be found in the archive
-
-        **Example:**
-
-        .. parsed-literal::
-
-            :swh_web_api:`origin/git/url/https://github.com/python/cpython/`
-    """
-    ori_dict = {
-        'id': int(origin_id) if origin_id else None,
-        'type': origin_type,
-        'url': origin_url
-    }
-    ori_dict = {k: v for k, v in ori_dict.items() if ori_dict[k]}
-    error_msg = 'Origin %s not found.' % \
-        (ori_dict.get('id') or ori_dict['url'])
-
-    return api_lookup(
-        service.lookup_origin, ori_dict,
-        notfound_msg=error_msg,
-        enrich_fn=_enrich_origin)
-
-
 @api_route(r'/origin/search/(?P<url_pattern>.+)/',
            'api-1-origin-search')
 @api_doc('/origin/search/')
@@ -569,3 +472,100 @@ def api_origin_intrinsic_metadata(request, origin_type, origin_url):
         service.lookup_origin_intrinsic_metadata, ori_dict,
         notfound_msg=error_msg,
         enrich_fn=_enrich_origin)
+
+
+@api_route(r'/origin/(?P<origin_url>.+)/', 'api-1-origin')
+@api_route(r'/origin/(?P<origin_id>[0-9]+)/', 'api-1-origin')
+@api_route(r'/origin/(?P<origin_type>[a-z]+)/url/(?P<origin_url>.+)/',
+           'api-1-origin')
+@api_doc('/origin/')
+@format_docstring(return_origin=DOC_RETURN_ORIGIN)
+def api_origin(request, origin_id=None, origin_type=None, origin_url=None):
+    """
+    .. http:get:: /api/1/origin/(origin_url)/
+
+        Get information about a software origin.
+
+        :param string origin_url: the origin url
+
+        {return_origin}
+
+        {common_headers}
+
+        **Allowed HTTP Methods:** :http:method:`get`, :http:method:`head`,
+            :http:method:`options`
+
+        :statuscode 200: no error
+        :statuscode 404: requested origin can not be found in the archive
+
+        **Example:**
+
+        .. parsed-literal::
+
+            :swh_web_api:`origin/git/url/https://github.com/python/cpython/`
+
+    .. http:get:: /api/1/origin/(origin_id)/
+
+        Get information about a software origin.
+
+        :param int origin_id: a software origin identifier
+
+        {return_origin}
+
+        {common_headers}
+
+        **Allowed HTTP Methods:** :http:method:`get`, :http:method:`head`,
+            :http:method:`options`
+
+        :statuscode 200: no error
+        :statuscode 404: requested origin can not be found in the archive
+
+        **Example:**
+
+        .. parsed-literal::
+
+            :swh_web_api:`origin/1/`
+
+    .. http:get:: /api/1/origin/(origin_type)/url/(origin_url)/
+
+        Get information about a software origin.
+
+        .. warning::
+
+            This endpoint is deprecated. You should use
+            :http:get:`/api/1/origin/(origin_url)/get/` instead.
+
+        :param string origin_type: the origin type (possible values are
+            ``git``, ``svn``, ``hg``, ``deb``, ``pypi``, ``npm``, ``ftp`` or
+            ``deposit``)
+        :param string origin_url: the origin url
+
+        {return_origin}
+
+        {common_headers}
+
+        **Allowed HTTP Methods:** :http:method:`get`, :http:method:`head`,
+            :http:method:`options`
+
+        :statuscode 200: no error
+        :statuscode 404: requested origin can not be found in the archive
+
+        **Example:**
+
+        .. parsed-literal::
+
+            :swh_web_api:`origin/git/url/https://github.com/python/cpython/`
+    """
+    ori_dict = {
+        'id': int(origin_id) if origin_id else None,
+        'type': origin_type,
+        'url': origin_url
+    }
+    ori_dict = {k: v for k, v in ori_dict.items() if ori_dict[k]}
+    error_msg = 'Origin %s not found.' % \
+        (ori_dict.get('id') or ori_dict['url'])
+
+    return api_lookup(
+        service.lookup_origin, ori_dict,
+        notfound_msg=error_msg,
+        enrich_fn=_enrich_origin)
\ No newline at end of file

This revision now requires changes to proceed.Jul 1 2019, 2:05 PM

No, that doesn't work.

There is no way to tell the difference between /origin/(?P<origin_url>.*)/ where origin_url is https://github.com/Foo/visits and /origin/(?P<origin_url>.*)/visits/ where origin_url is https://github.com/Foo.

I see, maybe we could add the /origin/visits/(?P<origin_url>.*)/ endpoint and deprecate the /origin/(?P<origin_url>.*)/visits/ ?

There is also numerous endpoints (even in browse) where this kind of issue will appear.

but then we're either using an inconsistent scheme, or have to change more half the URLs

Ok, so let's keep that diff that way. There is still the warning directive to parse by the apidoc module but this should be handled in another diff I think.

swh/web/api/views/origin.py
187	The warning directive is currently not parsed by the `apidoc` module. This must be implemented in order to add the deprecation notice to the endpoint descriptions.