Page MenuHomeSoftware Heritage

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

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

Details

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-jenkinsJenkins
Build 9244: arc lint + arc unit

Event Timeline

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.

anlambert added a subscriber: anlambert.

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.

This revision is now accepted and ready to land.Jul 1 2019, 2:46 PM
This revision was landed with ongoing or failed builds.Jul 2 2019, 1:10 PM
This revision was automatically updated to reflect the committed changes.