-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(aci): necessary flag changes #103087
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
fix(aci): necessary flag changes #103087
Conversation
mifu67
commented
Nov 10, 2025
- Set metric issue ingest/post-process flags to true so that metric issues can work for self-hosted
- Tie metric issue visibility to the UI flag so that rollout is smooth
- Tie new links to the UI flag
| }, | ||
| ) | ||
|
|
||
| @classmethod |
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.
@wedamija wanted to confirm that this is valid!
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.
Could we set released to True instead on the grouptype?
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 cannot, because the UI should not be visible yet :(
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.
Ahh, so we want to enable this in self-hosted?
Tbh, I wonder if we should set that feature flags we generate here to true in the self hosted config, instead of hardcoding true here. But if you plan to fully release it soon, it could be fine.
Why does this need to be enabled in self-hosted like this though? The issues won't be visible in the ui, so what's the value in allowing these other backend parts to run there?
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.
Current metric alerts are single processing through the workflow engine, so they will not work if metric issues are not not enabled. ACI is supposed to release soon, but I think either method is fine.
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.
I think that if you're planning to fully release this soon then I'm ok with this. Just make sure to clean up these methods later after you set released = True
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
- Set metric issue ingest/post-process flags to true so that metric issues can work for self-hosted - Tie metric issue visibility to the UI flag so that rollout is smooth - Tie new links to the UI flag
- Set metric issue ingest/post-process flags to true so that metric issues can work for self-hosted - Tie metric issue visibility to the UI flag so that rollout is smooth - Tie new links to the UI flag