search

alfred82santa / whalesong

Whalesong is an asyncio python library to manage WebApps remotely. Currently WhatsappWeb is implemented
https://whalesong.readthedocs.io
MIT License
50 stars 19 forks source link

allow multiple whalesong driver instances #96

Closed parthibd closed 5 years ago

parthibd commented 5 years ago

This fix allows whalesong to support multiple firefoxdriver instances by setting a random marionette port instead of hardcoding the port in code.

parthibd commented 5 years ago

What trouble will we run into if this was synchronous? I believe it needs to be synchronous since this is called only once during the driver initialization.

parthibd commented 5 years ago

This needs to be synchronous since it is called in the class constructor. I do understand your concern but I tested it and there is no such delay as in performance except for the few seconds it waits. Your suggested start_server in asyncio is not best fit for this since we have to wait for the port number before we can proceed any further.

parthibd commented 5 years ago

The only way we can use async in this context if we use factory pattern to init the driver. This would mean implementing a totally different way to initialize the driver. Do you think not using async in this case outweighs the benifits of using an async method?

parthibd commented 5 years ago

One other way we can proceed about this is to let the user assign one himself. Should I do that?

alfred82santa commented 5 years ago

Why you don't do it at this point? https://github.com/alfred82santa/whalesong/pull/96/files#diff-60f3e52bd1e97480f04fb0d902ecd524R72

In general, it's a bad practice to run sync external code (OS call) inside async code. Some times we need to do it, but I think it is not the case.

parthibd commented 5 years ago

Added a commit . Please review it.