Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad namespacing, imports breaks setuptools #345

Closed
dargueta opened this issue Aug 13, 2019 · 23 comments
Closed

Bad namespacing, imports breaks setuptools #345

dargueta opened this issue Aug 13, 2019 · 23 comments

Comments

@dargueta
Copy link
Contributor

fs.dropboxfs is implemented using fs as a namespace package. This causes it to misbehave when importing from fs for several reasons:

fs declares itself as a namespace package. According to the setuptools documentation here,

A) only namespace packages are supposed to contain the __import__('pkg_resources')...
B) they must not contain executable code aside from that import.

We violate both of those rules, thus resulting in weird exceptions like this:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/dargueta/dev/goodrx-data-scripts/airflow_dags/grx/third_party/dropbox.py", line 6, in <module>
    from fs.dropboxfs import DropboxFS
  File "/Users/dargueta/.pyenv/versions/2.7.15/envs/gds/lib/python2.7/site-packages/fs/dropboxfs/__init__.py", line 1, in <module>
    from .dropboxfs import DropboxFile
  File "/Users/dargueta/.pyenv/versions/2.7.15/envs/gds/lib/python2.7/site-packages/fs/dropboxfs/dropboxfs.py", line 12, in <module>
    from fs.base import FS
  File "/Users/dargueta/.pyenv/versions/2.7.15/envs/gds/lib/python2.7/site-packages/fs/base.py", line 24, in <module>
    from . import copy, errors, fsencode, iotools, move, tools, walk, wildcard
ImportError: cannot import name fsencode

cannot import name fsencode

Changing that line to these two fixes the problem:

from . import copy, errors, iotools, move, tools, walk, wildcard
from ._fscompat import fsencode

There's a similar issue in fs.test:

# Broken
from fs import ResourceType, Seek

# Works
from fs.enums import ResourceType, Seek

It appears these are the only two places in the repo we do this. Fixing these imports gets us a separate weird error in code using fs.dropboxfs but that may be unrelated.

@chfw
Copy link
Contributor

chfw commented Aug 13, 2019

for un-related reason, I renamed fs.gitpython to gitfs2.

@dargueta
Copy link
Contributor Author

@zopyx do you think it's feasible to rename fs.dropboxfs to fs-dropboxfs and bypass this whole issue? It'll still be broken for other namespaced packages but there don't seem to be many on PyPI that I see.

@chfw
Copy link
Contributor

chfw commented Aug 13, 2019

Here's my problem if fs.gitpython is used.

nose complains about the following:

======================================================================
ERROR: Failure: AttributeError (module 'fs' has no attribute 'gitpython')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jaska/github/moremoban/incubator/gitfs-env/lib/python3.7/site-packages/nose/failure.py", line 39, in runTest
    raise self.exc_val.with_traceback(self.tb)
  File "/Users/jaska/github/moremoban/incubator/gitfs-env/lib/python3.7/site-packages/nose/loader.py", line 407, in loadTestsFromName
    module = resolve_name(addr.module)
  File "/Users/jaska/github/moremoban/incubator/gitfs-env/lib/python3.7/site-packages/nose/util.py", line 322, in resolve_name
    obj = getattr(obj, part)
AttributeError: module 'fs' has no attribute 'gitpython'

So, I chose to avoid the problem.

@dargueta
Copy link
Contributor Author

I don't own the repo, so that's not an option for me. There's also the problem of it already being on PyPI.

@lurch
Copy link
Contributor

lurch commented Aug 14, 2019

I know nothing about namespace packages, but has this always been a problem? I'm wondering why it wasn't noticed before?

@zopyx
Copy link

zopyx commented Aug 14, 2019

Could you please how this is solved/handled in fs.webdavfs?

@willmcgugan
Copy link
Member

I recommend against publishing filesystems in the fs namespace. I can see the attraction of namespace packages, there is an appealing elegance about it. But in practice it's a real pain to work with. Particularly in development. Namespace packages don't play well with installing packages in editable mode for instance.

So the official recommendations don't use namespace packages. The only bit of magic required is the entry_points argument in setup.py which makes the filesystem available as an opener. In all other respects, you can publish a filesystem as any other Python module.

That said, there are several fs. namespace packages on PyPi and AFAIK they work just fine. So I'd be surprised if there was some fundamental breakage there.

I'll do a deep dive in to namespace packages at some point, so I can properly evaluate @dargueta 's PR. In the meantime, our resident namespaces expert is @althonos If he's not too busy, he might be able to shed some light on this.

@dargueta
Copy link
Contributor Author

dargueta commented Aug 14, 2019

I recommend against publishing filesystems in the fs namespace.

Not my repo: 😬

Also, webdavfs is broken unless I'm doing something wrong:

dargueta@tux ~ $ pip3 install fs.webdavfs
[...]
  Running setup.py install for fs.webdavfs ... done
Successfully installed fs.webdavfs-0.3.7 lxml-4.4.1 webdavclient2-0.1.1
dargueta@tux ~ $ ptpython
>>> import fs

>>> handle = fs.open_fs('webdav://admin:[email protected]:443/exist/webdav/db')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'fs' has no attribute 'open_fs'

module 'fs' has no attribute 'open_fs'

@althonos
Copy link
Member

The packages published under the fs namespace (like fs.sshfs) work fine as long as:

  • you don't install them in editable mode
  • the fs/__init__.py file is exactly the same as the one in Pyfilesystem2
  • the package must NOT be declared as a namespace package

There are some techniques like manual module path extension to avoid installing a package in editable mode while having your tests use the development version of the package.

@zopyx
Copy link

zopyx commented Aug 14, 2019

The fs.webdavfs package is not a namespace package. It only provides the top-level package webdav. The package name itself does not imply anything. And the documented usage for the fs.webdavfs module is using fs.open_fs().

@zopyx
Copy link

zopyx commented Aug 14, 2019

The fs.webdavfs package is not a namespace package. It only provides the top-level package webdav. The package name itself does not imply anything. And the documented usage for the fs.webdavfs module is using fs.open_fs().

The same is true for fs.dropbox.fs which only defines a top-level module dropboxfs.

@dargueta
Copy link
Contributor Author

If you try opening it using the dropbox:// protocol fs complains it can't find that module. I fought with it a lot and ended up going down a rabbit hole that lead me here.

@zopyx
Copy link

zopyx commented Aug 14, 2019

fs.dropboxfs is implemented using fs as a namespace package. This causes it to misbehave when importing from fs for several reasons:

@dargueta your assumption is wrong. There is no namespace definition in fs.dropboxfs - neither in the setup.py|.cfg files nor as part of the __init__.py file.

@dargueta
Copy link
Contributor Author

Okay well something's broken (see PyFilesystem/fs.dropboxfs#6). I now have no idea what. I'll close this for now. Thanks for your help!

@zopyx
Copy link

zopyx commented Aug 14, 2019

I see this here but this is obviously related to the entrypoint registration which is unrelated to namespace packages:

>>> import fs
>>> fs.open_fs("dropbox://xxxx")
Traceback (most recent call last):
  File "/Users/ajung/src/fs/lib/python3.7/site-packages/fs/opener/registry.py", line 136, in get_opener
    opener = entry_point.load()
  File "/Users/ajung/src/fs/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2411, in load
    return self.resolve()
  File "/Users/ajung/src/fs/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2417, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
ModuleNotFoundError: No module named 'dropboxfs'

@willmcgugan
Copy link
Member

@dargueta Your webdev example works for me. Fresh venv. Do you have the lastest (v2.4.10) PyFilesystem?

@dargueta
Copy link
Contributor Author

dargueta commented Aug 14, 2019

Yes. I really hope this isn't a platform thing. I see you're also using OSX. Never mind.

@dargueta
Copy link
Contributor Author

Tried a clean install and this happened...

dargueta@tux ~ $ pip3 install fs.dropboxfs
Error processing line 1 of /Users/dargueta/.pyenv/versions/3.7.3/lib/python3.7/site-packages/fs.dropboxfs-nspkg.pth:

  Traceback (most recent call last):
    File "/Users/dargueta/.pyenv/versions/3.7.3/lib/python3.7/site.py", line 168, in addpackage
      exec(line)
    File "<string>", line 1, in <module>
    File "<frozen importlib._bootstrap>", line 580, in module_from_spec
  AttributeError: 'NoneType' object has no attribute 'loader'

Remainder of file ignored
Looking in indexes: https://pypi.org/simple, https://engineering%40goodrx.com:****@goodrx.mycloudrepo.io/repositories/goodrx
Collecting fs.dropboxfs
[...]
Installing collected packages: fs, fs.dropboxfs
Successfully installed fs-2.4.10 fs.dropboxfs-0.2.2.post1

I am so confused now.

@willmcgugan
Copy link
Member

I am so confused now.

Ditto! I don't use Pyenv. I wonder if that's what's different about your environment?

@zopyx
Copy link

zopyx commented Aug 14, 2019

The entrypoint registration for fs.dropboxfs is working:

>>> fs_opener_registry.protocols
['userdata', 'userconf', 'sitedata', 'siteconf', 'usercache', 'userlog', 'ftp', 'mem', 'file', 'osfs', 'tar', 'temp', 'zip', 'dropbox']

However the opener registered for dropboxfs.opener can not be imported although the module is installed. However the distribution archive does not contain the dropboxfs module and its code:

drwx------ ajung/ajung       0 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/
-rw------- ajung/ajung    2938 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/PKG-INFO
-rw------- ajung/ajung     949 2019-08-14 16:56 fs.dropboxfs-0.2.2.post1/README.rst
drwx------ ajung/ajung       0 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/fs.dropboxfs.egg-info/
-rw------- ajung/ajung    2938 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/fs.dropboxfs.egg-info/PKG-INFO
-rw------- ajung/ajung     247 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/fs.dropboxfs.egg-info/SOURCES.txt
-rw------- ajung/ajung       1 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/fs.dropboxfs.egg-info/dependency_links.txt
-rw------- ajung/ajung      54 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/fs.dropboxfs.egg-info/entry_points.txt
-rw------- ajung/ajung      42 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/fs.dropboxfs.egg-info/requires.txt
-rw------- ajung/ajung       1 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/fs.dropboxfs.egg-info/top_level.txt
-rw------- ajung/ajung    1338 2019-08-14 17:05 fs.dropboxfs-0.2.2.post1/setup.cfg
-rwx------ ajung/ajung     104 2019-08-14 16:56 fs.dropboxfs-0.2.2.post1/setup.py

So the source distribution is just buggy. Perhaps it needs a MANIFEST or find_package() directive or something like that.

Your own issue is likely something with pyenv.

I would recommend using your standard Python 3 without pyenv and using a virtualenv created using python3 -m venv my_dir.

@dargueta
Copy link
Contributor Author

Perhaps it needs a MANIFEST or find_package() directive or something like that.

Oh woooooow I just figured it out -- the setup.cfg is wrong. packages: find: is in the metadata section, not options section where it's supposed to be.

Facepalmed so hard I might have a concussion...

@dargueta
Copy link
Contributor Author

dargueta commented Aug 14, 2019

Really sorry about wasting everyone's time.

@willmcgugan
Copy link
Member

Really sorry about wasting everyone's time.

Not at all! We all learned something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants