altered super call in CountedQueue init method (see #3073)

[bartkl]

The way how the CountedQueue.__init__ called its parent class's init method gave problems when trying to green the threading library using Eventlet.

See #3073 for the original discussion and purpose.

[bartkl]

Apparently, the Python 2.7.13 checks give this error:

`TypeError: super() argument 1 must be type, not classobj

I honeslty don't know why; the other classes call their super inits the same way. Also, locally I can parse the pipeline.py file just fine in Python 2.7.15.

Can someone help out? Thanks.

[NoahTheDuke]

This is why! Queue is probably an old style class, so try changing it to the new style and see if that fixes the issue.

jk I see that Queue is from six. I don't know what the solution is for that, haha.

[sampsyo]

Aha! That would explain it—looks like Queue is what's called an "old-style class," an unfortunate wart of Python 2.x that has many problems, including a lack of support for super.

The easiest way to do this might be to use something like this:

if six.PY2:
    queue.Queue.__init__(self, maxsize)
else:
    super(CountedQueue, self).__init__(maxsize)

which will at least make your trick work on Python 3+.

[wisp3rwind]

On a side note, I've recently been looking into cleaning up the pipeline code a little and revisiting the Queue invalidation in particular (in the context of #1957 and related issues). That being said, my current impression is that it's probably easiest to copy Python's Queue implementation to beets and modify it to our needs.

If I do indeed find the time to make a PR for this, it would also get rid of the new/old-style class problem.

[sampsyo]

Interesting, @wisp3rwind! I'd be interested to hear more—reimplementing threading.Queue in our own code sounds like a pretty major step. Is there a way to summarize what you'd change about it?

[wisp3rwind]

Interesting, @wisp3rwind! I'd be interested to hear more—reimplementing threading.Queue in our own code sounds like a pretty major step. Is there a way to summarize what you'd change about it?

In fact, the code for it is not that complex, and it could be stripped down significantly. The "cleanup" for now is a local branch that removes some code duplication in PipelineStage and subclasses and those changes to the queues. The point with the latter was to get rid of the hack that is used to invalidate queues on abort.

The larger picture is that I'd really like to implement this proposal with the end goal of being more robust on both intentionally aborted and crashed imports. Where robust is supposed to mean that it's as unlikely as possible that incomplete imports remain in the library (where completeness includes all plugin stages having run), and if that cannot be guaranteed in some cases, that at least the affected import tasks/items are logged. Which in turn would require pretty tight control over queues and pipeline stages, which with the current code is difficult to say the least.

Motivation is, even if not as relevant to me anymore right now, that I was bitten by incomplete imports when I initially started using beets and had aborted several imports (due to the kernel bug that made the ntfs3g driver block forever). While for most things this is not so important (it's easy to run fetchart and replaygain manually to fix up the incomplete items, and there are incremental imports (which might not be granular enough, though)) other errors might go unnoticed. For example, I use the third-party copyartifacts plugin that only copies files at the very end of the import.

[sampsyo]

Aha! Thanks for the detailed explanation. And thanks for looking into this thorny issue!

[bartkl]

Thanks for all the explanations and info.

Seems to me it's best I close this pull request, since it appears not to be as a quick pull as originally thought. I'll keep the change on my fork and keep experimenting, and who knows in the meanwhile @wisp3rwind will have improved the pipeline and fixed this issue along with it.