-
Notifications
You must be signed in to change notification settings - Fork 158
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
Adds environment & version to Deploy events #758
Changes from 7 commits
f859b02
d66404a
34e1b23
2ab2b62
d9d69a2
352361c
852fa3f
9f207e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,9 @@ type Empire struct { | |
// from the newly deployed image. | ||
ExtractProcfile ProcfileExtractor | ||
|
||
// Environment represents the environment this Empire server is responsible for | ||
Environment string | ||
|
||
// EventStream service for publishing Empire events. | ||
EventStream | ||
|
||
|
@@ -451,6 +454,9 @@ type DeploymentsCreateOpts struct { | |
// Image is the image that's being deployed. | ||
Image image.Image | ||
|
||
// Environment is the environment where the image is being deployed | ||
Environment string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably ok for now, but it'd probably be better in the long run if we had this on the Empire struct itself, so it works for non-github deployments. If I That means we'd need an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm - yeah, I guess actually taking the environment FROM the github deployment isn't actually all that useful. It's not like Empire runs multiple environments. Let me think about that - doesn't seem like it'd need to be super far reaching? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the initial version could just be used for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, took a first stab at addressing this. Let me know what you think. |
||
|
||
// Output is an io.Writer where deployment output and events will be | ||
// streamed in jsonmessage format. | ||
Output io.Writer | ||
|
@@ -464,6 +470,7 @@ func (opts DeploymentsCreateOpts) Event() DeployEvent { | |
if opts.App != nil { | ||
e.App = opts.App.Name | ||
} | ||
|
||
return e | ||
} | ||
|
||
|
@@ -481,7 +488,15 @@ func (e *Empire) Deploy(ctx context.Context, opts DeploymentsCreateOpts) (*Relea | |
return r, err | ||
} | ||
|
||
return r, e.PublishEvent(opts.Event()) | ||
event := opts.Event() | ||
event.Release = r.Version | ||
event.Environment = e.Environment | ||
// Deals with new app creation on first deploy | ||
if event.App == "" && r.App != nil { | ||
event.App = r.App.Name | ||
} | ||
|
||
return r, e.PublishEvent(event) | ||
} | ||
|
||
// ScaleOpts are options provided when scaling a process. | ||
|
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.
Nit, s/resource/resources/g