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

compatibility of IPFS plugin with Python 3 #2554

Merged
merged 2 commits into from Jul 14, 2020

Conversation

musoke
Copy link
Contributor

@musoke musoke commented May 10, 2017

I was looking to use the IPFS plugin, but it seemed not to be compatible with Python 3. I've made some small updates here that bring it to where the Python 2 version was.

However, this does not bring the IPFS plugin totally up to date; there is an issue with the command beet ipfs -g <hash> that became apparent when testing these changes. The other commands, -a, -i, -p, -l do seem to work though.

$ beet ipfs -g QmWZLpxqkcH6wwD1ZnpbhpSy6jhGZub42Vv24sh9rNPXFi
ipfs: Getting QmWZLpxqkcH6wwD1ZnpbhpSy6jhGZub42Vv24sh9rNPXFi from ipfs
No files imported from /home/user/src/beets/QmWZLpxqkcH6wwD1ZnpbhpSy6jhGZub42Vv24sh9rNPXFi
Traceback (most recent call last):
  File "/home/user/.local/bin/beet", line 9, in <module>
    load_entry_point('beets==1.4.4', 'console_scripts', 'beet')()
  File "/home/user/.local/lib/python2.7/site-packages/beets/ui/__init__.py", line 1228, in main
    _raw_main(args)
  File "/home/user/.local/lib/python2.7/site-packages/beets/ui/__init__.py", line 1215, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/home/user/.local/lib/python2.7/site-packages/beetsplug/ipfs.py", line 73, in func
    self.ipfs_get(lib, ui.decargs(args))
  File "/home/user/.local/lib/python2.7/site-packages/beetsplug/ipfs.py", line 151, in ipfs_get
    self.ipfs_get_from_hash(lib, query)
  File "/home/user/.local/lib/python2.7/site-packages/beetsplug/ipfs.py", line 171, in ipfs_get_from_hash
    shutil.rmtree(_hash)
  File "/usr/lib/python2.7/shutil.py", line 239, in rmtree
    onerror(os.listdir, path, sys.exc_info())
  File "/usr/lib/python2.7/shutil.py", line 237, in rmtree
    names = os.listdir(path)
OSError: [Errno 20] Not a directory: 'QmWZLpxqkcH6wwD1ZnpbhpSy6jhGZub42Vv24sh9rNPXFi'

(Note that this is a fake hash.)

This is also present with Python 2 and versions v1.4.3 or the current master (09ea54).

Presumably this was able to sneak in as the IPFS plugin is not tested by the CI.

cc @multikatt who introduced the plugin in #1397

@mention-bot
Copy link

@musoke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @multikatt, @wordofglass and @jrobeson to be potential reviewers.

@sampsyo
Copy link
Member

sampsyo commented May 11, 2017

Great; thank you! These were important fixes anyway.

@musoke
Copy link
Contributor Author

musoke commented May 11, 2017

OK, I'll fix those up and rebase.

Do you want me to open a separate issue/pull request for the bug?

@sampsyo
Copy link
Member

sampsyo commented May 11, 2017

Cool; thanks! For this, either a separate PR or an addendum to this one will be totally fine. Or you can just open the issue if you don't have time to propose a fix right now.

Paths were being constructed in a Python 3-incompatible way by concating
bytes and strings. Do this more carefully by encoding and decoding of
binary and strings.
In the future, just checking that a hash begins with "Qm" and has length
46 will likely not be sufficient.
@musoke musoke changed the title WIP: compatibility of IPFS plugin with Python 3 compatibility of IPFS plugin with Python 3 May 11, 2017
@musoke
Copy link
Contributor Author

musoke commented May 11, 2017

OK, done.

I opened an issue for the crash since I won't be fixing it in the next couple days.

@jtpavlock
Copy link
Contributor

Anything else that needs to get done here?

p.s. to keep the paper trail, #2555 is the opened issue referenced

@musoke
Copy link
Contributor Author

musoke commented Jul 14, 2020

This PR was complete at the time I wrote it. I was waiting for feedback/merging before fixing the other bug but that never happened.

sampsyo added a commit that referenced this pull request Jul 14, 2020
@sampsyo
Copy link
Member

sampsyo commented Jul 14, 2020

Apologies for letting this drop. Merged at last! ✨

@sampsyo sampsyo merged commit 8537d1e into beetbox:master Jul 14, 2020
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

Successfully merging this pull request may close these issues.

None yet

4 participants