Skip to content

fake_api [nfc]: Add delay option to connection.prepare #770

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

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Jun 28, 2024

This allows simulating a request that takes some time to return and consequently races with something else. Useful for situations like at #713 (comment) .

This is NFC because Future.delayed(Duration.zero, f) is exactly equivalent to Future(f). (The docs aren't real clear on this; but reading the implementations confirms they boil down to the very same Timer constructor call, and then the docs do seem to be trying to say that when read in light of that information.)

@chrisbobbe
Copy link
Collaborator

LGTM, thanks! I'll hold off on merging so @sm-sayedi can take a look; I see you've requested his review.

@sm-sayedi
Copy link
Collaborator

Looks good to me too. Thanks for this @gnprice.

This allows simulating a request that takes some time to return
and consequently races with something else.  For example here:
  zulip#713 (comment)

This is NFC because `Future.delayed(Duration.zero, f)` is exactly
equivalent to `Future(f)`.  (The docs aren't real clear on this;
but reading the implementations confirms they boil down to the
very same `Timer` constructor call, and then the docs do seem to be
trying to say that when read in light of that information.)
@gnprice gnprice force-pushed the pr-fake-api-delay branch from c1a32f7 to 4a906d5 Compare June 28, 2024 16:56
@gnprice gnprice merged commit 4a906d5 into zulip:main Jun 28, 2024
@gnprice
Copy link
Member Author

gnprice commented Jun 28, 2024

Merged. Thanks for the reviews!

@gnprice gnprice deleted the pr-fake-api-delay branch June 28, 2024 16:57
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