Skip to content

fix: self.command_executor instance in _update_command_executor#940

Merged
KazuCocoa merged 2 commits intomasterfrom
fix-connection
Dec 14, 2023
Merged

fix: self.command_executor instance in _update_command_executor#940
KazuCocoa merged 2 commits intomasterfrom
fix-connection

Conversation

@KazuCocoa
Copy link
Copy Markdown
Member

The new replaced instance was always RemoteConnection, which was not Appium's one. Use AppiumConnection if the executor was already made by AppiumConnection

# Override command executor
self.command_executor = RemoteConnection(executor, keep_alive=keep_alive)
# Override command executor.
if isinstance(self.command_executor, AppiumConnection): # type: ignore
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.

why should we ignore mypy here?

Copy link
Copy Markdown
Member Author

@KazuCocoa KazuCocoa Dec 13, 2023

Choose a reason for hiding this comment

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

I need to take a look later as a separate PR (to minimize each file change). For now, the below error kept rising without ignore. Maybe it needs a couple of lines' update with type hints.

appium/webdriver/webdriver.py:300: error: Cannot determine type of "command_executor"  [has-type]

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.

this looks like a mypy bug.
A similar issue has been reported here: python/mypy#13700

Lets keep it like that for now

@KazuCocoa KazuCocoa merged commit 17639ea into master Dec 14, 2023
@KazuCocoa KazuCocoa deleted the fix-connection branch December 14, 2023 06:42
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