Skip to content
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

feat: added client.wait_for_start() #87

Merged
merged 1 commit into from
Oct 17, 2024
Merged

Conversation

ErikBjare
Copy link
Member

@ErikBjare ErikBjare commented Oct 17, 2024

Important

Adds wait_for_start method to ActivityWatchClient to wait for server start with timeout handling.

  • New Method:
    • Adds wait_for_start(timeout: int = 10) to ActivityWatchClient in client.py.
    • Attempts to retrieve server info repeatedly until successful or timeout.
    • Raises exception if server does not start within the timeout period.

This description was created by Ellipsis for ade23b5. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to ade23b5 in 16 seconds

More details
  • Looked at 25 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. aw_client/client.py:383
  • Draft comment:
    Catching req.exceptions.ConnectionError is too broad and might catch unrelated exceptions. Consider using requests.ConnectionError instead.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment suggests a change in exception handling, which is relevant to the logic of the new method. However, the suggestion seems incorrect because req is an alias for requests, making the current exception handling correct. The comment does not provide strong evidence for a necessary change.
    I might be missing some context about the use of req as an alias, but the import statement clearly shows requests is aliased as req. The comment seems to misunderstand this.
    Given the import statement, the current exception handling is correct, and the comment's suggestion is unnecessary.
    The comment should be deleted because it incorrectly suggests a change that is not needed due to the aliasing of requests as req.

Workflow ID: wflow_feg8zFC0PDWvykp0


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@ErikBjare ErikBjare merged commit b693873 into master Oct 17, 2024
6 of 7 checks passed
@ErikBjare ErikBjare deleted the dev/wait-for-start branch October 17, 2024 16:29
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.

1 participant