UmbrellaDish patch importable fido 1#149
Open
UmbrellaDish wants to merge 2 commits intoopenpreserve:mainfrom
Open
UmbrellaDish patch importable fido 1#149UmbrellaDish wants to merge 2 commits intoopenpreserve:mainfrom
UmbrellaDish wants to merge 2 commits intoopenpreserve:mainfrom
Conversation
*Not tested, yet* Take it as an issue with source code attachment ready to merge if error-free. Moved argument processing and sys.argv checks out of main() into `if __name__ == '__main__'` clause. Applied dest='other_name' for certain names that have reserved meaning in python or which are to short to be meaningful. `main()` accepts directly passed keyword arguments (in theory they may be positional, but rather don't), so the caller does not need argparse.
Spotted more left-overs to fix after lunch
Contributor
|
No longer involved in this project (but still lurkin' tho'), and this is what I came up with back then: http://wiki.opf-labs.org/display/KB/FIDO+Python+workflow+implementation+tips HTH |
Contributor
|
Rebasing this should bring it up to date with the master code base and get the build tests passing. |
Member
|
@adamfarquhar this is probably worth investigating at least. The OP makes some good points and it might be they could be addressed to some degree by refactoring toward an API and library. |
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
4 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.
In the endeavor of integration in our python3-based preservation workflow, FIDO is a diva in need of extra treatment. It is at last the main() function that combines argument processing boilerplate as is to be done when called from the command line on one hand and purpose-specific things on the other.
Currently we need to redirect of sys.stdin and sys.stderr to an in-memory StringIO, shadow-resetting sys.argv to a bogus value in order to prevent print_help() from being called, and then feeding the StringIO thing to a csv reader instance. This makes our code gather pity when shown around, though it feels yet better in comparison to a plain sub-process (python calling python as a sub-process? Not quite easy to even type such a thought).
Maybe main() should even be a yielding generator when called without handle_matches argument. Everything needed in command-line tool mode should be restricted in scope, i.e. it belongs into the
if __name__ = '__main__'clause.Even if you intended the Fido class to be used in any integrated environment, main() contains too much logic that one might want and would be required to re-implement.
The changes have not yet been tested as in the first place I would like to know if the direction of my proposal draws your accordance.