Use output of rpm --eval command, in case import of rpm module fails#139
Merged
Use output of rpm --eval command, in case import of rpm module fails#139
Conversation
Redesign utils module to prevent circular dependency Resolves #117
Contributor
irushchyshyn
left a comment
irushchyshyn
left a comment
There was a problem hiding this comment.
Looks good.
I believe if those functions are used only in metadata_extractors.py then they should not have been in utils in the first place, so moving them there is fine and fixes circular dependency issue. But we'll never be able to use settings in utils, oh well:)
How about adding some tests for get_default_save_path and rpm_eval?
| """ | ||
| license = [] | ||
| for classifier in trove: | ||
| if 'License' in classifier != -1: |
Contributor
There was a problem hiding this comment.
I believe != -1 part is redundant?
| ['rpm', '--eval', macro], | ||
| stdout=subprocess.PIPE).communicate()[0].strip() | ||
| except OSError: | ||
| logger.error('Failed to get value of {0} rpm macro'.format( |
Contributor
There was a problem hiding this comment.
I think this should be warning for the user, also saying that the default path will be used.
irushchyshyn
approved these changes
Sep 26, 2017
Contributor
irushchyshyn
left a comment
irushchyshyn
left a comment
There was a problem hiding this comment.
Thanks for the fixes. Looks good.
Please squash the fixup and merge at will.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Order of the methods to get DEFAULT_PKG_SAVE_PATH is following:
Some functions from utils had to be moved elsewhere to prevent circular dependency between utils and settings modules.