Potential fix for imports #1167
Closed
Conversation
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
added 3 commits
October 7, 2020 17:43
The end goal is to get rid of imports style
import package.module
as that is incompatible with vendoring. The secondary goal is to
preserve the ability to fully namespace things (so to use
tuf.module.something in the code).
This commit defines the basic set of library imports.
exceptions
formats
keydb
roledb
settings
import sig
These will be available under tuf namespace after "import tuf"
There's also a set of client imports
download
mirrors
These will be available under client namespace after
"from tuf import client".
These sets do not include
log (because importing it has side effects)
repository_tool
repository_lib
developer_tool
Signed-off-by: Jussi Kukkonen <[email protected]>
These changes are not required: the old forms work fine but these changes get the tests inline with the rest of the code. Signed-off-by: Jussi Kukkonen <[email protected]>
Importing the log module has side-effects so it does not happen as part of the default set in __init__.py: Remove unused log imports and update the remaining uses to "from tuf import log as tuf_log". Signed-off-by: Jussi Kukkonen <[email protected]>
updater module cannot be included in 'client' default modules if updater module also loads 'client' (as python2 fails to import it). We do want to load client default modules in updater to be able to use namespacing, e.g. client.download.safe_download(). So don't include updater in default modules: this means the examples should use e.g. from tuf.client import updater or from tuf.client.updater import Updater Signed-off-by: Jussi Kukkonen <[email protected]>
|
closing this: it was useful as a test for the workaround I'm going to use in pip , but will never be a real solution we'd want to merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Maybe fixes issue #1165 ?
This style of import:
import tuf.downloadis incompatible with vendoring tool (typically vendoring modifies
from X import Yintofrom vendored.path.X import Ybut this can't be succesfully done with our style).So I'd like to talk about changing the import style. Unfortunately just switching to the style
from tuf import downloadmeans massive code churn as the local name changes fromtuf.downloadtodownload: at least 700 lines of change and significant chance of namespace collision.This PR is a exploration of another way: using package imports to keep the code mostly as is.
Description of the changes being introduced by the pull request:
import tuf.somethingThis should be 100% backwards compatible: all import styles still work.
The important changes in functionality are:
Please verify and check that the pull request fulfills the following
requirements: