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

Patch Studio API update timinig issue #4619

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Sep 4, 2023

Related to #3574

After pushing an experiment there is a delay before it becomes available via the new Studio GET endpoint.

There were two possible solutions that came to mind.

  1. Continually ping the API until the experiment becomes available.
  2. Assume that if the experiment is on the Git remote it will be available in Studio by the time the link icon is shown.

I went with option 2 as option 1 seemed to be more brittle.

Demo

Screen.Recording.2023-09-05.at.12.34.34.pm.mov

Note: We can update this later if we run into issues. The code is clearly signposted.

@mattseddon mattseddon added the product PR that affects product label Sep 4, 2023
@mattseddon mattseddon self-assigned this Sep 4, 2023
this.experiments.assumePushed(shas)
}

private waitForStudioUpdate() {
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] I tried to combine the waitForStudioUpdate & waitForRemoteUpdate methods but the code was a lot harder to read.

}

const [, { lsRemoteOutput }] = await Promise.all([
this.waitForStudioUpdate(),
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] If we don't wait for this the assumePushed shas can be overwritten.

@mattseddon mattseddon marked this pull request as ready for review September 5, 2023 02:43
Copy link
Contributor

@julieg18 julieg18 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!

@codeclimate
Copy link

codeclimate bot commented Sep 5, 2023

Code Climate has analyzed commit 18b2e18 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 93.9% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.0% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit ddcd7f7 into main Sep 5, 2023
3 checks passed
@mattseddon mattseddon deleted the patch-timing-issue branch September 5, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants