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

Adds a command to perform signals testing to the CLI #5285

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

mvilanova
Copy link
Contributor

This pull request includes several changes to the src/dispatch/cli.py file, focusing on import optimizations, command adjustments, and the addition of a new performance testing utility.

Import Optimizations:

  • Reorganized imports to ensure proper order and avoid redundancy. (src/dispatch/cli.py) [1] [2]

Command Adjustments:

  • Adjusted the import statements within the dispatch_scheduler function to ensure necessary modules are imported correctly. (src/dispatch/cli.py) [1] [2]

New Performance Testing Utility:

  • Added a new perf-test command to the signals_group for performance testing of signal instances. This includes options for the number of instances, number of workers, API endpoint, API token, and project. (src/dispatch/cli.py)

@mvilanova mvilanova added the enhancement New feature or request label Oct 3, 2024
src/dispatch/cli.py Outdated Show resolved Hide resolved
@kevgliss
Copy link
Contributor

kevgliss commented Oct 3, 2024

Should the perf test be an actual pytest instead of a command? I'm not quite sure how you're using the command or the exact baseline you're trying to guarantee. But if we are worried about performance regressions, we should figure out how to run this automatically, I think.

@kevgliss kevgliss closed this Oct 3, 2024
@kevgliss kevgliss reopened this Oct 3, 2024
@mvilanova
Copy link
Contributor Author

Should the perf test be an actual pytest instead of a command? I'm not quite sure how you're using the command or the exact baseline you're trying to guarantee. But if we are worried about performance regressions, we should figure out how to run this automatically, I think.

We've been using the command to generate synthetic signals for testing purposes during the development of new features. We could rename the command to remove any ambiguity. I also like the idea of having a test that does that. Perhaps something we can work on during Q1.

@mvilanova mvilanova merged commit f53427d into master Oct 3, 2024
14 checks passed
@mvilanova mvilanova deleted the enhancement/cli-signals-perf-test branch October 3, 2024 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants