fm-orchestrator

Clone
Source Code
GIT

#1152 Change exception information for errors when parsing a modulemd file
Merged 5 years ago by mprahl. Opened 5 years ago by vmaljulin.
vmaljulin/fm-orchestrator FACTORY-3909  into  master

file modified
+13 -1 
@@ -699,7 +699,19 @@ 
 

          # If the modulemd was v1, it will be upgraded to v2
 

          mmd.upgrade()
 

      except Exception:
 

-         error = 'The following invalid modulemd was encountered: {0}'.format(yaml)
 

+         if is_file:
 

+             error = 'The modulemd {} is invalid. Please verify the syntax is correct'.format(
 

+                 os.path.basename(yaml)
 

+             )
 

+             if os.path.exists(yaml):
 

+                 with open(yaml, 'rt') as yaml_hdl:
 

+                     log.debug('Modulemd content:\n%s', yaml_hdl.read())
 

+             else:
 

+                 error = 'The modulemd file {} not found!'.format(os.path.basename(yaml))

The contents of this are still raised in raise UnprocessableEntity(error), thus exposing the path to the user.

 
+                 log.error('The modulemd file %s not found!', yaml)
 

+         else:
 

+             error = 'The modulemd is invalid. Please verify the syntax is correct'
 

+             log.debug('Modulemd content:\n%s', yaml)
 

          log.exception(error)
 

          raise UnprocessableEntity(error)

file modified
+5 -2 
@@ -33,6 +33,7 @@ 
 

  import hashlib
 

  import pytest
 

  from module_build_service.utils import to_text_type
 

+ import re
 

  

  from tests import app, init_data, clean_database, reuse_component_init_data
 

  from tests import read_staged_data
 
@@ -972,7 +973,8 @@ 
 

              {'branch': 'master', 'scmurl': 'https://src.stg.fedoraproject.org/modules/'
 

                  'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'}))
 

          data = json.loads(rv.data)
 

-         assert data['message'].startswith('The following invalid modulemd was encountered') is True
 

+         assert re.match(r'The modulemd .* is invalid\. Please verify the syntax is correct',
 

+                         data['message'])
 

          assert data['status'] == 422
 

          assert data['error'] == 'Unprocessable Entity'
 

  
@@ -1596,7 +1598,8 @@ 
 

          data = json.loads(rv.data)
 

  

          assert data['error'] == 'Unprocessable Entity'
 

-         assert data['message'].startswith('The following invalid modulemd was encountered')
 

+         assert re.match(r'The modulemd .* is invalid\. Please verify the syntax is correct',
 

+                         data['message'])
 

  

      @pytest.mark.parametrize('api_version', [1, 2])
 

      @patch('module_build_service.auth.get_user', return_value=import_module_user)

This fixes #1149

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

Can you add a new line after Modulemd content:? That way the yaml is on a new line in the logs and it's easier to read.

Can you log the path where it wasn't found?

It might be useful to move this check up in the function. It makes sense to me to verify the path exists before running mmd = Modulemd.Module().new_from_file(yaml). It'd also make the code a bit cleaner.

rebased onto 0f7c515652d39ed71bbce232a4decfddeccee8fb
5 years ago

Can you add a new line after Modulemd content:? That way the yaml is on a new line in the logs and it's easier to read.

Done

Can you log the path where it wasn't found?

Done

It might be useful to move this check up in the function. It makes sense to me to verify the path exists before running mmd = Modulemd.Module().new_from_file(yaml). It'd also make the code a bit cleaner.

It wouldn't make the code much cleaner because there still will be a check before log.debug as long as you don't want to have an exception in the except block.

The path should be logged, but not returned to the user.

rebased onto f6a4bef
5 years ago

The path should be logged, but not returned to the user.

Done

The contents of this are still raised in raise UnprocessableEntity(error), thus exposing the path to the user.

The contents of this are still raised in raise UnprocessableEntity(error), thus exposing the path to the user.

But there is only the basename in the exception message. Or should I delete it also?

@vmaljulin you're right. I misread it. This looks good.

Pull-Request has been merged by mprahl
5 years ago
Changes Summary 2
+13 -1
file changed
module_build_service/utils/submit.py
+5 -2
file changed
tests/test_views/test_views.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.