-
Notifications
You must be signed in to change notification settings - Fork 131
Add environment lock/unlock/locked handling #96
Add environment lock/unlock/locked handling #96
Conversation
Looks pretty good. Can you run |
@@ -30,6 +30,21 @@ | |||
Resque.inline = true | |||
end | |||
|
|||
config.around do |example| |
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's this stuff for?
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.
It's to prevent tests from messing with the default Heaven.redis
instance. Here I'm switching to a different database number on localhost
.
We could alternatively use something like fakeredis or mock_redis during testing.
@atmos All set on the Maybe something like this would be more concise? def run!
unless receivers.detect { |receiver| receiver.new(event, guid, data).run! }
Rails.logger.info "Unhandled event type, #{event}."
end
end
private
def receivers
[LockReceiver, DeploymentReceiver, DeploymentStatusReceiver, StatusReceiver]
end Receivers that can't handle the event can return nil or false. |
@@ -30,6 +30,14 @@ | |||
Resque.inline = true | |||
end | |||
|
|||
config.around do |example| | |||
original = Heaven.redis | |||
Heaven.redis = Redis.new(:url => "redis://localhost:6379/3") |
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.
Can we use an environmental variable for this? like ENV['BOXEN_REDIS_URL'] || 'redis://localhost:6379'
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 main thing here is the /3
database number. 3
could really be any number beyond the default of 0
. I don't see I way to extract the currently used redis url and tack on a different database number.
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.
Looks like we can do something like this instead:
config.around do |example|
original = Heaven.redis.client.db
Heaven.redis.select(3)
example.run
Heaven.redis.flushall
Heaven.redis.select(original)
end
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 could use the last database index of 15
which I doubt anyone is going to use.
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.
Yeah, let's try the redis select thing on 15 and see how everything is working.
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.
Just pushed up a commit. Seems fine on my machine.
…ed-handling Add environment lock/unlock/locked handling
Can you send another pr documenting the behavior of locking? |
I just fixed it in something that reads a little easier than what you proprosed in 86fdcb6 |
Hi! Commands |
@willdurand Looks like the GitHub deployment webhook payload has changed since this feature was implemented. We'll have to update the mapping here. Happy to do that in a PR this week if someone doesn't get to it first. Here's where you would find the current payload structure:
|
This PR adds support for
deploy:lock
anddeploy:unlock
tasks.These tasks are intercepted and handled separate from any provider.
When an attempt to deploy to a locked environment is made, we'll send up an error deployment status with a description including who locked the environment.
Addresses #55