Skip to content

feat: respect the given executor#844

Merged
KazuCocoa merged 10 commits intomasterfrom
allow-executor
Apr 1, 2023
Merged

feat: respect the given executor#844
KazuCocoa merged 10 commits intomasterfrom
allow-executor

Conversation

@KazuCocoa
Copy link
Copy Markdown
Member

Got a request to use the given executor if the first argument was the instance as same as Selenium does.

Example usage is in the README.

init_args_for_pool_manager = {'retries': urllib3.util.retry.Retry(total=3, connect=3, read=False)}
custom_appium_connection = CustomAppiumConnection(
# keep alive has different route to set init args for the pool manager
keep_alive=True,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Noticed the old method did not work when keep_alive was enabled, so modified to pass the arguments as the init

@KazuCocoa KazuCocoa requested review from mykola-mokhnach and removed request for ki4070ma and mykola-mokhnach March 28, 2023 17:23
README.md Outdated
appium_executor = AppiumConnection(remote_server_addr='http://127.0.0.1:4723',
init_args_for_pool_manage=init_args_for_pool_manage)

options = XCUITestOptions().load_capabilities({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

load_capabilities was created rather for compatibility with older stuff. Consider using appropriate setters and getters instead

):

# Need to call before super().__init__ in order to pass arguments for the pool manager in the super.
if init_args_for_pool_manager is None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could be simplified to self._init_args_for_pool_manager = init_args_for_pool_manager or {}

@KazuCocoa KazuCocoa merged commit 2c92c04 into master Apr 1, 2023
@KazuCocoa KazuCocoa deleted the allow-executor branch April 1, 2023 20:27
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.

2 participants