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

Implementation of websocket API for importer #3073

Open
bartkl opened this issue Nov 4, 2018 · 5 comments
Open

Implementation of websocket API for importer #3073

bartkl opened this issue Nov 4, 2018 · 5 comments
Labels

Comments

@bartkl
Copy link
Contributor

@bartkl bartkl commented Nov 4, 2018

I'm playing around with websockets with the ultimate goal of building a web importer frontend for the beets importer process. Currently I'm trying to get a proof of concept to work using Flask-SocketIO with eventlet. For this it's necessary to monkey patch several stdlib modules to 'green' them so they're compatible with eventlet.

However, when I perform the monkey patch I get an error:

    queues = [CountedQueue(queue_size) for i in range(queue_count)]
  File "/opt/cucumba/venv/lib/python3.6/site-packages/beets/util/pipeline.py", line 95, in __init__
    queue.Queue.__init__(self, maxsize)
  File "/opt/cucumba/venv/lib/python3.6/site-packages/eventlet/green/Queue.py", line 15, in __init__
    super(Queue, self).__init__(maxsize)
TypeError: super(type, obj): obj must be an instance or subtype of type

After some analysis and trial and error, I got things to work by altering the init method of CountedQueue in pipeline.py:

95c95
<         queue.Queue.__init__(self, maxsize)
---
>         super(CountedQueue, self).__init__(maxsize)

That's nice, but I have a few questions still:

  1. Is there any good reason why the super() call was not used in the first place, as seems general practice in the file (and community?)
  2. I honestly still don't precisely understand the exact differences in the original call and my replaced super() call. Could someone elaborate on that?
  3. I don't understand why the original situation leads to the specific error.
  4. Is this adjustment okay for the pipeline (somewhere in the future)?

Trying to learn and contribute :).

Thanks,
Bart

@sampsyo

This comment has been minimized.

Copy link
Member

@sampsyo sampsyo commented Nov 5, 2018

Wow! This seems like an incredibly cool project! I'm really excited to see where you go with it.

  1. I don't see any reason why I didn't use super there. This change of yours looks like a clear improvement. 😃
  2. They should be exactly the same—except when you change the base class. That is, super(CountedQueue, self) gets a "version" of self with methods that look up in a different starting class. That does the same thing as calling methods explicitly on the base class as named.
  3. The trouble is that "greening" the library changed the base class of CountedQueue, but the call to the super class's __init__ method wasn't aware of that. So it ended up calling __init__ in the "wrong" class.
  4. Yes! Please go right ahead and open a pull request with this change by itself; we should merge it right away.

Thanks!!

@sampsyo sampsyo added the discussion label Nov 5, 2018
@bartkl

This comment has been minimized.

Copy link
Contributor Author

@bartkl bartkl commented Nov 5, 2018

Thanks for your feedback :).

I'm on very new ground here, so it might take me some time to get this done properly. Currently I'm reading up on coroutine based asynchronous programming, and differences between threading and green threads, etcetera. I'm gathering understanding little by little, but it's quite overwhelming still.
Especially how the multiple threads of the pipeline are greened and taken up into eventlet's event loop (as I understand happens) is really not clear yet to me.

That lack of understanding gives rise to a very specific problem: how do I make the importer wait for a choice that I made in the browser (or whatever client)? Because of the async nature of things, it's not really normal to want some function to block and wait for an event to happen.

If you -or anyone else for that matter- has tips on how I can improve my knowledge best here, please let me know. Also, I might want to ask for help more. Somehow I tend to fele insecure doing that, which is a shame.

I'll see if I can create a pull request today with that small change.

Thanks!
Bart

@bartkl

This comment has been minimized.

Copy link
Contributor Author

@bartkl bartkl commented Nov 6, 2018

@sampsyo Feels appropriate to rename this issue to something like Implementation of websocket API for importer, wouldn't you agree?

@sampsyo

This comment has been minimized.

Copy link
Member

@sampsyo sampsyo commented Nov 6, 2018

Sure; sounds good.

@bartkl bartkl changed the title Question about pipeline.Queue init method Implementation of websocket API for importer Nov 8, 2018
@bartkl

This comment has been minimized.

Copy link
Contributor Author

@bartkl bartkl commented Nov 11, 2018

Okay I think I could really benefit from reading up on some fundamental theory on threading, async event-driven programming and the pipeline design pattern.

Could anyone recommend good books or other sources (blogs, videos) that will help me out? I've found some interesting talks on YouTube, but I notice not everything sticks too well because I lack fundamental knowledge. I'll post this question on the discussion board as well.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.