Skip to content

api-review-2 #501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pavelfeldman opened this issue Feb 12, 2021 · 8 comments
Closed

api-review-2 #501

pavelfeldman opened this issue Feb 12, 2021 · 8 comments

Comments

@pavelfeldman
Copy link
Member

pavelfeldman commented Feb 12, 2021

  • expose_function/binding missing Callback params in docs / docstrings
  • route.continue_ vs resume
  • consider get_ prefixes. pending async considerations.
@mxschmitt
Copy link
Member

mxschmitt commented Feb 13, 2021

Also thought about to introduce upstream a new Error called BrowsersExecutableNotFoundError as a type (like Error and TimeoutError) which then can be used in Python to better check if the user has to run python -m playwright install. This would give better error reporting as in #477 was catched as such an error but it was actually a browser launch error.

@kumaraditya303
Copy link
Contributor

Also the playwright error should be namespaced with Playwright i.e. PlaywrightError and PlaywrightTimeoutError which is less confusing than the current Error and TimeoutError @pavelfeldman

@kumaraditya303
Copy link
Contributor

Also #507

@mxschmitt
Copy link
Member

Also the playwright error should be namespaced with Playwright i.e. PlaywrightError and PlaywrightTimeoutError which is less confusing than the current Error and TimeoutError @pavelfeldman

I agree there, had that also in mind.

@kumaraditya303
Copy link
Contributor

@mxschmitt Is playwright python docker image public now?

@kumaraditya303
Copy link
Contributor

@mxschmitt @pavelfeldman In the README for async API, Jupyter Notebook is suggested but instead we should recommend python's builtin asyncio repl which can be accessed by python -m asyncio:
Example

$ python -m asyncio
asyncio REPL 3.9.1 (tags/v3.9.1:1e5d33e, Dec  7 2020, 17:08:21) [MSC v.1927 64 bit (AMD64)] on win32
Use "await" directly instead of "asyncio.run()".
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> 

@kumaraditya303
Copy link
Contributor

@mxschmitt May be we should change the way sync api works with something like #439 (comment)

@pavelfeldman
Copy link
Member Author

This is now done.

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

No branches or pull requests

3 participants