-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Replay Verify scheduler #15103
Replay Verify scheduler #15103
Conversation
⏱️ 2h 54m total CI duration on this PR
🚨 1 job on the last run was significantly faster/slower than expected
|
2d4bcf2
to
bdb718b
Compare
c776074
to
b40e99a
Compare
b40e99a
to
78dfd06
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪄
testsuite/replay-verify/main.py
Outdated
from enum import Enum | ||
|
||
|
||
# Hyperparameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what? is there any machine learning involved? 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the ones that are supposed to passed by config
The rest should be fixed
# Check if the environment variable already exists | ||
for env_var in container["env"]: | ||
if env_var["name"] == name: | ||
env_var["value"] = value | ||
return | ||
|
||
# If it doesn't exist, add it | ||
container["env"].append({"name": name, "value": value}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what, you can't just "upsert"?
(I've forgotten Python)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a vector of map/dict and each dict is {"name": name, "value": value}. The format is from k8s config.
serviceAccountName: default | ||
restartPolicy: Never # Pod restarts only if it fails | ||
containers: | ||
- name: replay-verify-worker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably gonna wanna generate this in code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is updated dynamically instart()
function. This file serves as a template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might wanna add replay-verify as a tool in poetry so you can poetry run the tool
https://gist.github.com/perryjrandall/cc38e1a47c49d7da807a512ce7283796
78dfd06
to
490666f
Compare
5311ad3
to
5f4e5e8
Compare
26d9c75
to
b49aa77
Compare
b49aa77
to
65a65fc
Compare
65a65fc
to
957ee8f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
Description
Next step is to put it on a real cluster and add it to github workflow
How Has This Been Tested?
Tested on my local kube cluster
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist