enable changing of url relative to selenium #1548
Conversation
danielklim
requested review from
alexcjohnson,
Marc-Andre-Rivet and
rpkyle
as code owners
|
Thanks @danielklim - just to be clear, can you show an example of using these new attributes? Also please switch to double quotes to appease the linter (or install |
|
@alexcjohnson dev merged in and here is an explanation/demo of the issue Let me know if you need anything else -- thanks! |
|
@danielklim thanks for the great demo, and apologies for being slow to get back to this. Does the demo represent how you would use this in practice? I'm glad that with this update the necessary functionality exists, but it bothers me that you need to synchronize code in the calling function ( |
|
@alexcjohnson I think it's a great idea to add command line arguments as you suggested to avoid having to hardcode the remote url! I'm a bit slammed with other work right now so can't really do it myself but it should be a pretty quick fix for anyone who is actively working on the code base; or you could just merge in my original MR since it does technically do what it's supposed to and create a future todo for the cmd line args. |
|
Hi @T4rk1n - is this one still relevant or has it gone stale? |
dash/testing/application_runners.py
Outdated
| @@ -52,6 +52,8 @@ class BaseDashRunner(object): | |||
| """Base context manager class for running applications.""" | |||
|
|
|||
| def __init__(self, keep_open, stop_timeout): | |||
| self.scheme = "http" | |||
| self.host = "localhost" | |||
There was a problem hiding this comment.
The new attributes should be added as argument with their defaults.
dash/testing/application_runners.py
Outdated
| @@ -92,7 +94,7 @@ def __exit__(self, exc_type, exc_val, traceback): | |||
| @property | |||
| def url(self): | |||
| """The default server url.""" | |||
| return "http://localhost:{}".format(self.port) | |||
| return "{}://{}:{}".format(self.scheme, self.host, self.port) | |||
There was a problem hiding this comment.
Resolve conflict by using f-string.
|
thanks @T4rk1n - I'll add it to the pile for the next cycle. |
|
Glad to see this getting merged in! |
gvwilson
added
P2
Currently, there is no way to use Selenium grid in a separate container/pod because
BaseDashRunnerassumes that the Dash server will be on localhost relative to the Selenium host. In other words, for a setup where the Dash app lives in container1 and Selenium lives in container 2, pytest can be invoked withpytest --remote-url http://container2:4444but it will fail because Selenium in container2 will attempt to connect to localhost rather than container1 (since url@BaseDashRunner is hardcoded to localhost).This MR adds
hostandschemeattributes to BaseDashRunner so that the user can better specify the location of the app relative to Selenium.