Skip to content

Add SDK method support#6

Merged
MrCreosote merged 4 commits intomainfrom
dev-client
Nov 5, 2025
Merged

Add SDK method support#6
MrCreosote merged 4 commits intomainfrom
dev-client

Conversation

@MrCreosote
Copy link
Member

No description provided.

@MrCreosote MrCreosote requested a review from briehl November 3, 2025 21:58
3s is not enough if the image is pulling
@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.33%. Comparing base (8de6e2b) to head (df751df).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #6      +/-   ##
==========================================
+ Coverage   97.75%   98.33%   +0.58%     
==========================================
  Files           1        1              
  Lines          89      120      +31     
==========================================
+ Hits           87      118      +31     
  Misses          2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Currently no test where the _call method returns a list of arguments
because I can't think of any services, dynamic or normal, that do that
Copy link
Member

@briehl briehl left a comment

Choose a reason for hiding this comment

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

Just a couple questions / comments. I'm approving anyway.

import requests as _requests
import os as _os
from urllib.parse import urlparse as _urlparse
import time as _time
Copy link
Member

Choose a reason for hiding this comment

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

Why the underscore imports? Sticking to previous convention?

Copy link
Member Author

@MrCreosote MrCreosote Nov 5, 2025

Choose a reason for hiding this comment

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

yeah, that's how it is in the original client. it's actually not a bad practice as it keeps the namespace cleaner for introspection and other use cases



# tested this manually by shortening _EXP_BACKOFF_MS and adding printouts below
def _get_next_backoff(backoff_index: int = 1):
Copy link
Member

Choose a reason for hiding this comment

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

Doing return typings here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

* Serializing set and frozenset
* Methods that return a list vs. a single value (save_objects).
"""
# TODO add test for service that returns > 1 value. Not sure if any services do this
Copy link
Member

Choose a reason for hiding this comment

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

AFAICT there aren't any. But it's possible to have them, so I guess just leave the TODO for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, if it comes to it I'll mock one. But if no services are doing it, then not testing it isn't really a huge issue

@MrCreosote MrCreosote merged commit 3ea1d14 into main Nov 5, 2025
5 of 6 checks passed
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