parser - Return non-zero code for wiki errors. #658

Merged
merged 7 commits into from Aug 16, 2016

Conversation

Projects
None yet
4 participants
Contributor

josepht commented Jul 14, 2016

LP:#1602394

Signed-off-by: Joe Talbott joe.talbott@ubuntu.com

Contributor

josepht commented Jul 15, 2016

@sergiusens I can't add or remove labels. Is that a power you can grant me?

snapcraft/internal/parser.py
- return master_parts_list
+ return (master_parts_list, errors)
@elopio

elopio Aug 5, 2016

Member

I like that we are returning the number of errors, but in python we shouldn't return a tuple like in go.
Instead, if errors > 0, raise an exception. You can add an attribute to the WikiError class that's number_of_errors, so you can extract it later.

snapcraft/internal/parser.py
def _process_index(output):
# XXX: This can't remain in memory if the list gets very large, but it
# should be okay for now.
master_parts_list = OrderedDict()
+ errors = dict(wiki=0)
@elopio

elopio Aug 5, 2016

Member

I got lost with this. Do we have any other kind of errors? Why isn't it just a counter?

@josepht

josepht Aug 9, 2016

Contributor

The thinking is that we'd have at least two classes of errors; 1. wiki entry errors (i.e. malformed yaml, missing fields, etc.) 2. snapcraft.yaml errors that make constructing the fully parsed part impossible. For either one we want to be able to continue processing the subsequent entries. I'll look into handling this with exceptions.

Collaborator

sergiusens commented Aug 9, 2016

Looks good, can you add a unit test showing this wiki parsing problem count?

josepht added some commits Jul 14, 2016

parser - Return non-zero code for wiki errors.
LP:#1602394

Signed-off-by: Joe Talbott <joe.talbott@ubuntu.com>
Collaborator

sergiusens commented Aug 10, 2016

retest this please

snapcraft/internal/parser.py
part_name = data.get('project-part')
if part_name is not None and part_name in master_parts_list:
logger.warning("Duplicate part found in wiki: {}".format(
part_name))
- return
+ raise InvalidEntryError('duplicate entry: {}'.format(entry))
@kyrofa

kyrofa Aug 12, 2016

Member

You capitalized the one above. Should this one be so as well?

@kyrofa

kyrofa Aug 12, 2016

Member

Also, the warning and exception messages are redundant. Perhaps it would be better to only raise here, and log in the catcher of the exception instead?

@@ -722,11 +727,6 @@ def test_carriage_returns(self, mock_get, mock_get_origin_data):
origin: lp:snapcraft-parser-example
description: example main
project-part: main\r
----
-maintainer: Jim Doe <jim.doe@example.com>
@kyrofa

kyrofa Aug 12, 2016

Member

Who the heck was Jim, anyway? John's brother?

josepht added some commits Aug 12, 2016

@sergiusens sergiusens merged commit bda3b10 into snapcore:master Aug 16, 2016

3 checks passed

autopkgtest integration Success
Details
autopkgtest snaps Success
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@josepht josepht deleted the josepht:bugs/1602394 branch Aug 19, 2016

kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017

parser - Return non-zero code for wiki errors. (#658)
LP:#1602394

Signed-off-by: Joe Talbott <joe.talbott@ubuntu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment