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

fix: fix timing function cross-platform bug #14919

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

DerekMaggio
Copy link
Contributor

Overview

Closes https://opentrons.atlassian.net/browse/EXEC-406

Use a timing function universal to Windows, Mac, and Linux when running on Windows and Mac
This will allow performance metrics tests to run locally for devs
The production implementation will always use Linux

Test Plan

  • Create test that patches _get_timing_function to return universal timing function and make sure that works
  • Realized I missed timing synchronous functions so I added that too

Changelog

  • In robot_context_tracker.py create a _get_timing_function which imports the correct package based on os

Review requests

None

Risk assessment

low

@DerekMaggio DerekMaggio requested a review from a team as a code owner April 16, 2024 17:22
@DerekMaggio DerekMaggio requested a review from a team April 16, 2024 17:22
@DerekMaggio DerekMaggio changed the title Exec 406 time function bug fix: fix timing function cross-platform bug Apr 16, 2024

P = ParamSpec("P")
R = TypeVar("R")


def _get_timing_function() -> Callable[[], int]:
Copy link
Member

Choose a reason for hiding this comment

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

let's refactor this so that it only does the logic check once at import time rather than every time it's called

@DerekMaggio DerekMaggio requested a review from sfoster1 April 16, 2024 18:04
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

LGTM

@DerekMaggio DerekMaggio merged commit d77bbb7 into edge Apr 16, 2024
5 checks passed
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
# Overview

Closes https://opentrons.atlassian.net/browse/EXEC-406

Use a timing function universal to Windows, Mac, and Linux when running
on Windows and Mac
This will allow performance metrics tests to run locally for devs
The production implementation will always use Linux

# Test Plan

- Create test that patches _get_timing_function to return universal
timing function and make sure that works
- Realized I missed timing synchronous functions so I added that too


# Changelog

- In robot_context_tracker.py create a _get_timing_function which
imports the correct package based on os

# Review requests

None
# Risk assessment

low
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