-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: move experiment calculator to batch service #1160
Conversation
94f091f
to
246bab6
Compare
|
||
const ( | ||
experimentLockPrefix = "experiment-calculate:" | ||
experimentLockTTL = time.Minute |
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.
Depending on the number of events, it could take more than 1 minute to finish analyzing the data.
It will be safer if we set the default value to 10 minutes.
Also, can you make this configurable? It will be nice to pass the config via values.yaml.
) | ||
|
||
const ( | ||
experimentLockPrefix = "experiment-calculate:" |
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.
experimentLockPrefix = "experiment-calculate:" | |
experimentLockKind = "experiment_lock" |
} | ||
|
||
// getLockKey generates the lock key for a specific experiment | ||
func (el *ExperimentLock) getLockKey(environmentID, experimentID string) string { |
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.
func (el *ExperimentLock) getLockKey(environmentID, experimentID string) string { | |
func (el *ExperimentLock) newLockKey(environmentID, experimentID string) string { |
|
||
// getLockKey generates the lock key for a specific experiment | ||
func (el *ExperimentLock) getLockKey(environmentID, experimentID string) string { | ||
return fmt.Sprintf("%s%s:%s", experimentLockPrefix, environmentID, experimentID) |
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.
return fmt.Sprintf("%s%s:%s", experimentLockPrefix, environmentID, experimentID) | |
return fmt.Sprintf("%s:%s:%s", environmentID, experimentLockKind, experimentID) |
return el.lock.Unlock(ctx, lockKey, value) | ||
} | ||
|
||
// getLockKey generates the lock key for a specific experiment |
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.
// getLockKey generates the lock key for a specific experiment | |
// newLockKey generates the lock key for a specific experiment |
454c510
to
741f24e
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.
Thank you!
This PR will move experiment calculator to batch service.
Key Changes
This pull request includes several changes that focus on removing the
experiment-calculator
component and adding a newhttpstan
service. The most important changes include the removal of the mainexperimentcalculator.go
file, updates to the Helm charts, and modifications to the dependencies in thego.mod
file.Removal of
experiment-calculator
component:cmd/experimentcalculator/experimentcalculator.go
: Removed the entire file, including the main function and related imports.manifests/bucketeer/charts/experiment-calculator/Chart.yaml
: Removed the Helm chart configuration for theexperiment-calculator
.manifests/bucketeer/charts/experiment-calculator/templates/_helpers.tpl
: Removed helper templates for theexperiment-calculator
.manifests/bucketeer/charts/experiment-calculator/templates/deployment.yaml
: Removed the deployment configuration for theexperiment-calculator
.manifests/bucketeer/charts/experiment-calculator/templates/NOTES.txt
: Removed the notes for theexperiment-calculator
deployment.manifests/bucketeer/charts/experiment-calculator/.helmignore
: Removed the.helmignore
file for theexperiment-calculator
.Addition of
httpstan
service:manifests/bucketeer/charts/batch/templates/deployment.yaml
: Added configuration for the newhttpstan
service, including image details, ports, and health checks.manifests/bucketeer/charts/batch/values.yaml
: Added values for thehttpstan
service, such as image repository, tag, port, and health path.Dependency updates:
go.mod
: Addedgithub.com/google/uuid v1.6.0
to the dependencies and removed it from the indirect dependencies. [1] [2]