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

chore: fix wrong stressor init in validator #1844

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rootfs
Copy link
Contributor

@rootfs rootfs commented Nov 14, 2024

this is to fix the previous PR that didn't get full tested. This change is verified with the following config:

log_level: warn # Logging level, defaults is warn

remote:
  host: my-vm
  username: root
  pkey: /tmp/vm_ssh_key

metal:
  vm:
    pid: 123456 # Process ID for the KVM process running on metal

prometheus:
  job:
    vm: vm  # Job name for virtual machine metrics, default is vm
    metal: metal  # Job name for metal metrics, default is metal

  url: http://localhost:9090 # Prometheus server URL
  rate_interval: 20s  # Rate interval for Promql, default is 20s, typically 4 x $scrape_interval
  steps: 3s  # Step duration for Prometheus range queries

stressor:
  total_runtime_seconds: 600  # Total runtime in seconds for the stress test
  curve_type: default  # Curve type for the stress test, default is linear

validations_file: ./validations.yaml  # Path to the validations file, default is ./validations.yaml

Copy link

github-actions bot commented Nov 14, 2024

🤖 SeineSailor

Here's a concise summary of the pull request changes:

Summary: This pull request enhances stress testing capabilities in the validator by introducing total_runtime_seconds and curve_type parameters to the remote.run_script method and Stressor class initialization. The changes also update the validator configuration to include a "stressor" section with default values for these new parameters.

Key Modifications:

  1. Added total_runtime_seconds and curve_type parameters to remote.run_script method call.
  2. Updated Stressor class initialization to include total_runtime_seconds and curve_type parameters.
  3. Introduced a "stressor" section in the validator configuration with default values for total_runtime_seconds (1200) and curve_type ("default").

Impact: These changes facilitate more comprehensive stress testing and correct the stressor initialization in the validator, enabling more accurate and efficient testing.

Observations/Suggestions:

  • It would be beneficial to include additional documentation or comments explaining the purpose and usage of the new parameters and configuration section.
  • Consider adding tests to validate the correctness of the stress testing functionality with the new parameters.
  • Review the default values for total_runtime_seconds and curve_type to ensure they are suitable for most use cases.

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