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

Add DCDO flag for contact rollups #43681

Merged
merged 2 commits into from
Nov 18, 2021
Merged

Add DCDO flag for contact rollups #43681

merged 2 commits into from
Nov 18, 2021

Conversation

megcrenshaw
Copy link
Contributor

@megcrenshaw megcrenshaw commented Nov 18, 2021

This adds a DCDO flag for contact rollups if we need to turn them off during HOC. I set the flag in production, but not in the other environments (it shouldn't be run in other environments) besides my local environment.

The flag name is 'contact_rollups_active'. It is currently set to true––to turn off contact rollups, set the flag to false.

Links

Testing story

I tested on my local environment, setting the DCDO flag in my local dashboard-console. You can also see the flag in AWS.

Deployment strategy

Follow-up work

Question: Do I need to add a task for removing this flag after HOC?

Privacy

Security

Caching

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@megcrenshaw megcrenshaw requested review from a team and bethanyaconnor November 18, 2021 14:16
@bethanyaconnor
Copy link
Contributor

Do I need to add a task for removing this flag after HOC?

Honestly, I think we should keep it. Hopefully we never have to turn this job off but it's good to have the option!

@@ -21,4 +21,9 @@ ensure
contact_rollups.report_results
end

def main
return false unless DCDO.get('contact_rollups_active', true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny nit: I don't think you need the false here and can just say return unless ...

Copy link
Contributor

@bethanyaconnor bethanyaconnor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a really tiny nit but otherwise LGTM!

@megcrenshaw
Copy link
Contributor Author

Do I need to add a task for removing this flag after HOC?

Honestly, I think we should keep it. Hopefully we never have to turn this job off but it's good to have the option!

Sounds good. I suppose if the flag gets deleted, we'd have a problem (could change it to only disabling if there's a flag that says to disable it?)

@bethanyaconnor
Copy link
Contributor

if the flag gets deleted, we'd have a problem (could change it to only disabling if there's a flag that says to disable it?)

I think it would still be okay. If the flag got deleted out of AWS, it would (presumably) be nil and the default would be used instead.

I realized that I never thought of sending you the actual code for DCDO flags 🤦‍♀️ You may have already seen it but here is how that fallback comes into play

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@megcrenshaw megcrenshaw merged commit 3d204e0 into staging Nov 18, 2021
@megcrenshaw megcrenshaw deleted the contact-rollups-flag branch November 18, 2021 19:52
@megcrenshaw
Copy link
Contributor Author

if the flag gets deleted, we'd have a problem (could change it to only disabling if there's a flag that says to disable it?)

I think it would still be okay. If the flag got deleted out of AWS, it would (presumably) be nil and the default would be used instead.

I realized that I never thought of sending you the actual code for DCDO flags 🤦‍♀️ You may have already seen it but here is how that fallback comes into play

Ah, thank you, Bethany! Super helpful

@davidsbailey
Copy link
Member

Do I need to add a task for removing this flag after HOC?

Honestly, I think we should keep it. Hopefully we never have to turn this job off but it's good to have the option!

+1 for keeping the flag, however it seems like we will still want to remember to turn the flag back on after HOC. @tess323 how do you think we should track this, for this and future years? It seems like the perfect kind of thing to go into whatever the HOC equivalent of our "ship curriculum" doc, which I know we might not quite have yet.

@megcrenshaw
Copy link
Contributor Author

Do I need to add a task for removing this flag after HOC?

Honestly, I think we should keep it. Hopefully we never have to turn this job off but it's good to have the option!

+1 for keeping the flag, however it seems like we will still want to remember to turn the flag back on after HOC. @tess323 how do you think we should track this, for this and future years? It seems like the perfect kind of thing to go into whatever the HOC equivalent of our "ship curriculum" doc, which I know we might not quite have yet.

The flag is on right now, and we aren’t planning to turn it off unless we’re desperate (from what I understand). So I think we just need to create a task to turn it on if we need to turn it off during HoC

@davidsbailey
Copy link
Member

The flag is on right now, and we aren’t planning to turn it off unless we’re desperate (from what I understand). So I think we just need to create a task to turn it on if we need to turn it off during HoC

Ohh, I see! OK, that plan sounds good to me. Do we know what symptoms would prompt us to turn off this flag? or if we aren't responsible for that part of it, is this something we should communicate to the infrastructure team so they can put it in their playbook?

@megcrenshaw
Copy link
Contributor Author

The flag is on right now, and we aren’t planning to turn it off unless we’re desperate (from what I understand). So I think we just need to create a task to turn it on if we need to turn it off during HoC

Ohh, I see! OK, that plan sounds good to me. Do we know what symptoms would prompt us to turn off this flag? or if we aren't responsible for that part of it, is this something we should communicate to the infrastructure team so they can put it in their playbook?

Good questions. I'm going to tap in @tess323 and @clareconstantine as I think they both have more info about who made this original request.

@tess323
Copy link

tess323 commented Nov 22, 2021

Hey team - I have this included in our HOC launch doc. Most likely we will be offboarding ownership of rollups in spring-summer - so I will make a sure to include this in a handoff doc.

I agree it makes sense to keep the flag around for next year or if we need to pause again in the future.

After Suresh did some investigation on the rollups - he strongly believes we should turn this off for the week of HOC. I have tasks in this sprint (and next), but lets plan to flip the flag on 12/3 and turn it back on early in the week of 12/15

@megcrenshaw
Copy link
Contributor Author

Sounds good, Tess! Thanks for the documentation here and adding documentation for the future

snickell pushed a commit that referenced this pull request Feb 3, 2024
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.

None yet

4 participants