Skip to content

Conversation

@ajpotts
Copy link
Contributor

@ajpotts ajpotts commented Apr 30, 2025

This PR address some issues brought up in #3964 regarding the handling of running mode in the benchmarks. Some improvements:

  • Options are added to pytest_adoption that cover the environment variables set in benchmark.ini. This clarifies that directly specified arguments take precedent, followed by the value of the corresponding environment variable, and the default value is used as last resort.
  • The server=pytest.host is used in start_arkouda_server(host=pytest.host, numlocales=nl, port=pytest.port) and ak.connect(server=pytest.host, port=pytest.port, timeout=pytest.timeout) to ensure the tests connect to the same server that was started during the tests.

Part of #3964: Refactor benchmark handling of running mode

@ajpotts ajpotts requested a review from vasslitvinov April 30, 2025 18:16
Copy link
Contributor

@vasslitvinov vasslitvinov left a comment

Choose a reason for hiding this comment

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

This is a good step forward, however it addresses only part of #3964. I suggest to keep 3964 open.

@ajpotts ajpotts marked this pull request as ready for review May 1, 2025 13:54
@ajpotts ajpotts marked this pull request as draft May 1, 2025 14:30
@ajpotts ajpotts changed the title Closes #3964: Refactor benchmark handling of running mode Part of #3964: Refactor benchmark handling of running mode May 1, 2025
@ajpotts ajpotts force-pushed the 3964_Refactor_benchmark_handling_of_running_mode branch 3 times, most recently from 0a2c6ab to 6d3aa4d Compare May 1, 2025 18:42
@ajpotts ajpotts requested a review from vasslitvinov May 1, 2025 18:44
@ajpotts ajpotts force-pushed the 3964_Refactor_benchmark_handling_of_running_mode branch from 6d3aa4d to 0f743c7 Compare May 1, 2025 19:06
@ajpotts ajpotts marked this pull request as ready for review May 1, 2025 19:31
@ajpotts ajpotts force-pushed the 3964_Refactor_benchmark_handling_of_running_mode branch 2 times, most recently from 9797880 to f9dea9c Compare May 1, 2025 21:37
@ajpotts ajpotts force-pushed the 3964_Refactor_benchmark_handling_of_running_mode branch from f9dea9c to 149d713 Compare May 2, 2025 13:59
@ajpotts ajpotts enabled auto-merge May 2, 2025 14:00
Copy link
Contributor

@jaketrookman jaketrookman 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

@ajpotts ajpotts added this pull request to the merge queue May 2, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 2, 2025
@ajpotts ajpotts added this pull request to the merge queue May 2, 2025
Merged via the queue into Bears-R-Us:master with commit 51b6fcb May 3, 2025
25 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.

4 participants