Skip to content

Conversation

@KazuCocoa
Copy link
Member

This feature is the same as https://github.com/projectxyzio/web2driver#direct-connect-urls and https://www.rubydoc.info/github/appium/ruby_lib_core/Appium/Core/Driver#direct_connect-instance_method

Use case:

  • If a user has a proxy server such as SeleniumGrid as below, an Appium client communicate with Appium servers via the proxy every command.
    # via proxy server in session creation
    client <--> proxy server <---> appium server <-> devices
                                                 `-> appium server <-> devices
    
    • Then, the communication takes additional seconds between the client and servers. The proxy server is if in the US and the Appium server is in Europ, the time increases more.
    • To reduce the delay, we can distribute the proxy server to each reason, but it increases the complexity of the proxy server management.
  • If we can make such communication directly with the client and servers, it can reduce the time easily.
    # communicate directly after the session creation
    client <--------> appium server <-> devices
                       `---> appium server <-> devices
    
    • The first create session command takes the additional time, but the following commands can communicate with servers directly with less communication latency.
      • We can manage a bunch of custom urls in a client side, but it increases management complexity in the client side.

I've added direct_connection flag, and it is false by default, to be able to handle the flag.

Different as SeleniumGrid, mobile apps tie to physical locations. Measuring performance including network latency is necessary to manage devices in various locations, for example. In such case, one proxy server and Appium servers in various locations make easy to manage the complexity. The proxy has device selection logic. The client only sends a session creation command to the proxy, no many URLs.


cc @jlipps

By.IMAGE = MobileBy.IMAGE
By.CUSTOM = MobileBy.CUSTOM

def _update_command_executor(self, keep_alive):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably check that these values actually exist. just because someone has set the direct_connection param doesn't mean the server supports it, right?

up to you whether you think we should throw an error if one of these values is not present, or just silently ignore the request to direct connect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch, I forgot it...

I'll show warning or error message rather than stopping test script since whether a server cannot handle the following requests or not depend on them.

path=path
)

logger.info('Update request endpoint to %s', executor)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

)

if self.command_executor is not None:
if self.command_executor is not None: # pylint: disable=access-member-before-definition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could probably use hasattr to avoid pylint warning

if (not {direct_protocol, direct_host, direct_port, direct_path}.issubset(set(self.capabilities))):
message = 'Direct connect capabilities from server were:\n'\
'{protocol}: {protocol_get}, {host}: {host_get}, {port}: {port_get}, {path}: {path_get}'.format(
protocol=direct_protocol, protocol_get=self.capabilities.get(direct_protocol, ''),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capabilities is already a dictionary, so we could in theory to something like **self.capabilities

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to show a blank in no key case like below, so I changed to for style

directConnectProtocol: 'http'
directConnectHost: 'localhost2'
directConnectPort: '4800'
directConnectPath: ''

@KazuCocoa KazuCocoa merged commit 1830af4 into appium:master Feb 27, 2019
@KazuCocoa KazuCocoa deleted the add-direct-connect branch February 27, 2019 00:55
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.

3 participants