[Proposal] "Continuous Deployment Hours"

[jayzalowitz]

Continuous deployment is great, but deploy hours are great things to enforce in this system.

Would yall be willing to add something that is basically business hours?

@cloudcustodian has some good examples https://cloudcustodian.io/docs/quickstart/offhours.html

[benlangfeld]

We implement this as a pre-deploy check in the shipit.yml. I can drop an example shortly. It works well for us.

[benlangfeld]

Apologies for the delay, but here is our implementation:

In shipit.yml:

review:
  checks:
    - bundle exec bin/prod_deploy_time_limit

In bin/prod_deploy_time_limit:

#!/usr/bin/env ruby

# frozen_string_literal: true

require "active_support/all"
require "tzinfo"
require "tzinfo/data"

tz = Time.find_zone("America/New_York")
now = tz.now

puts "Considering current time #{now}"

if now.saturday? || now.sunday?
  puts "Deploys not allowed at the weekend"
  exit 1
end

if now.friday? && now > tz.parse("16:30 pm")
  puts "Deploys not allowed on Friday after 16:30 Eastern"
  exit 1
end

puts "No time restriction active. Deploys allowed."
exit 0

[casperisfine]

I think this is an interesting feature, and I do remember some interest for it.

It can indeed be done with a pre-deploy check, but it could make sense as a more polished feature.

I'm just not sure wether it would make more sense as a global configuration or as a part of shipit.yml (or both?)

[benlangfeld]

I'm just not sure wether it would make more sense as a global configuration or as a part of shipit.yml (or both?)

In our case this is applied only to a single stack, not even globally to a whole repo, let alone globally to all repos. I think it needs to go in shipit.yml.

[benlangfeld]

@powerhome Will contribute this feature once #1102 is merged.

[casperisfine]

once #1102 is merged.

Damn, I really need to review that, but it's really too big. Any way you could break it up? e.g. by not submitting the API just yet etc?

[benlangfeld]

@casperisfine We tried to submit it in pieces toward the start of the year (eg #967) and you asked that we continue until it delivers value. I'm not sure where the middle ground is.

[casperisfine]

I think there is a misunderstanding. My concern at that time was that it was adding code without adding any feature, so it was hard to understand the goal.

My concern now it's that it's too big to wrap my head around it. The diff is literally +8,501 −19, the middle would be a diff on the low thousands that adds a behavior that can be tested. So I'm asking wethers that are part of it that are somewhat self contained and that could be reviewed independently to make it easier.

Not sure how it is at powerhome, but here we almost never review anything that big.

But whatever, I might try to break it up myself.

[benlangfeld]

@casperisfine Several thousand of those lines are sample github notifications in fixtures, more than half is tests, and I have yet to identify any subset of the change which is truly self-contained and simultaneously meets the criteria of being small and easily reviewable while also adding useful behaviour.

[benlangfeld]

@casperisfine We could split out things like the repository CRUD UI and the tracking of PR data, those would each be small and independently reviewable, but would not deliver any end-user value.

[casperisfine]

Ok, I guess I'll just deal with it. I'll reserve my Monday morning.

[davidcornu]

@casperisfine would you be willing to accept a contribution that adds this feature? We have this use case at work and seems like something that'd be useful to others. Happy to write up an implementation plan first so we don't have to hash out specifics in a PR.

[casperisfine]

👋

Yeah, I think it's a feature that makes sense.