#497 Change error from 502 to 404 when Koji build not found
Merged 4 years ago by vmaljulin. Opened 4 years ago by vmaljulin.
vmaljulin/greenwave FACTORY-4255  into  master

file modified
+3
@@ -388,6 +388,9 @@ 

          {"type":"RemoteRule"}]. Do not use this parameter along with `decision_context`.

      :statuscode 200: A decision was made.

      :statuscode 400: Invalid data was given.

+     :statuscode 404: No Koji build found

+     :statuscode 502: Error while querying Koji to retrieve the SCM URL

+     :statuscode 504: Timeout while querying an upstream

      """  # noqa: E501

      data = request.get_json()

      if data:

file modified
+2 -3
@@ -14,7 +14,7 @@ 

  from urllib.parse import urlparse

  import xmlrpc.client

  from flask import current_app

- from werkzeug.exceptions import BadGateway

+ from werkzeug.exceptions import BadGateway, NotFound

  

  from greenwave.cache import cached

  from greenwave.request_session import get_requests_session
@@ -137,8 +137,7 @@ 

  

  def retrieve_scm_from_koji_build(nvr, build, koji_url):

      if not build:

-         raise BadGateway(

-             'Failed to find Koji build for "{}" at "{}"'.format(nvr, koji_url))

+         raise NotFound('Failed to find Koji build for "{}" at "{}"'.format(nvr, koji_url))

  

      source = build.get('source')

      if not source:

@@ -5,7 +5,7 @@ 

  

  import pytest

  import mock

- from werkzeug.exceptions import BadGateway

+ from werkzeug.exceptions import BadGateway, NotFound

  

  import greenwave.app_factory

  from greenwave.resources import (
@@ -42,7 +42,7 @@ 

      nvr = 'foo-1.2.3-1.fc29'

      build = {}

      expected_error = 'Failed to find Koji build for "{}" at "{}"'.format(nvr, KOJI_URL)

-     with pytest.raises(BadGateway, match=expected_error):

+     with pytest.raises(NotFound, match=expected_error):

          retrieve_scm_from_koji_build(nvr, build, KOJI_URL)

  

  

This fixes #415

Signed-off-by: Valerij Maljulin vmaljuli@redhat.com

rebased onto 36b52319176db24e981aecaf7bb4d4522f80ad88

4 years ago

This could potentially break someone's code.

Can you add documentation for the status code? Seems that still only the 200 and 400 are in docs.

This could potentially break someone's code.
Can you add documentation for the status code? Seems that still only the 200 and 400 are in docs.

+1 let's update the docs to avoid confusion.

Agreed on the doc, otherwise +1
And as a reminder: we need to release a major version since it's not backward compatible.

rebased onto d2e3f23

4 years ago

Pull-Request has been merged by vmaljulin

4 years ago