Skip to content

Conversation

@shamrin
Copy link
Contributor

@shamrin shamrin commented Oct 23, 2025

No description provided.

@shamrin shamrin requested a review from tamirse October 23, 2025 11:13
pricing: Optional[Pricing] = None,
coupon: Optional[str] = None) -> Instance:
coupon: Optional[str] = None,
*,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

* is a keyword-only argument marker, probably a good idea to do it for all new arguments.

https://docs.python.org/3/glossary.html#term-parameter

Comment on lines 171 to 194
# Wait for instance to enter provisioning state with timeout
MAX_WAIT_TIME = 60 # Maximum wait time in seconds
POLL_INTERVAL = 0.5 # Time between status checks

start_time = time.time()
interval = min(initial_interval, max_interval)
deadline = time.monotonic() + max_wait_time
while True:
instance = self.get_by_id(id)
if instance.status != InstanceStatus.ORDERED:
return instance

if time.time() - start_time > MAX_WAIT_TIME:
now = time.monotonic()
if now >= deadline:
raise TimeoutError(
f"Instance {id} did not enter provisioning state within {MAX_WAIT_TIME} seconds")
f"Instance {id} did not enter provisioning state within {max_wait_time:.1f} seconds")

time.sleep(POLL_INTERVAL)
time.sleep(min(interval, deadline - now))
interval = min(interval * backoff_coefficient, max_interval)
Copy link

@bemyak bemyak Oct 23, 2025

Choose a reason for hiding this comment

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

It's not a big change, but it creates less variables and I guess it's a bit easier to read:

        import itertools # ← at the top
        # Wait for instance to enter provisioning state with timeout
        start_time = time.monotonic()
        for i in itertools.count():
            instance = self.get_by_id(id)
            if instance.status != InstanceStatus.ORDERED:
                return instance

            time_left = max_wait_time - (time.monotonic() - start_time)
            if time_left < 0:
                raise TimeoutError(
                    f"Instance {id} did not enter provisioning state within {max_wait_time:.1f} seconds")

            interval = initial_interval * backoff_coefficient ** i
            time.sleep(min(interval, max_interval, time_left))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've used some ideas from your version.

@shamrin shamrin force-pushed the feature/instance-create-retry-params branch from 98d562d to 68ef37a Compare October 23, 2025 12:20
@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.36%. Comparing base (657b3b1) to head (68ef37a).
⚠️ Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
datacrunch/instances/instances.py 33.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
- Coverage   87.98%   87.36%   -0.62%     
==========================================
  Files          21       21              
  Lines        1124     1148      +24     
==========================================
+ Hits          989     1003      +14     
- Misses        135      145      +10     

☔ 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.

@shamrin shamrin merged commit 0d775c6 into master Oct 23, 2025
16 checks passed
@shamrin shamrin deleted the feature/instance-create-retry-params branch October 23, 2025 13:33
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.

3 participants