-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Enable Backfill in E2E #13524
Enable Backfill in E2E #13524
Conversation
@@ -0,0 +1,38 @@ | |||
package flags |
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 the reason to break flags out into a separate package?
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.
We need to import the backfill flag to our feature package, since we want it as part of --dev
. If you dont have a separate package, build fails due to dependency cycles
@@ -283,7 +283,7 @@ func (node *BeaconNode) Start(ctx context.Context) error { | |||
// on our features or the beacon index is a multiplier of 2 (idea is to split nodes | |||
// equally down the line with one group having feature flags and the other without | |||
// feature flags; this is to allow A-B testing on new features) | |||
if !config.TestFeature || index%2 == 0 { | |||
if !config.TestFeature || index != 1 { |
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.
The comment above indicates this was supposed to be only for multiples of 2 - which one is intended?
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.
Will fix the comment, but the current intent is to only have 1 node running as a control rather than splitting it down the line
What type of PR is this?
E2E Improvement
What does this PR do? Why is it needed?
This enables backfill for our E2E runs by adding it to our
dev
mode.Which issues(s) does this PR fix?
N.A
Other notes for review