-
Notifications
You must be signed in to change notification settings - Fork 8
Improvements to deployment of production items #695
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
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,44 @@ | ||
Class SourceControl.Git.DeploymentLog Extends %Persistent [ Owner = {%Developer} ] | ||
{ | ||
|
||
Property Token As %String [ InitialExpression = {$System.Util.CreateGUID()} ]; | ||
|
||
Property StartTimestamp As %TimeStamp; | ||
|
||
Property EndTimestamp As %TimeStamp; | ||
|
||
Property HeadRevision As %String; | ||
|
||
Property Status As %Status; | ||
|
||
Storage Default | ||
{ | ||
<Data name="DeploymentLogDefaultData"> | ||
<Value name="1"> | ||
<Value>%%CLASSNAME</Value> | ||
</Value> | ||
<Value name="2"> | ||
<Value>Token</Value> | ||
</Value> | ||
<Value name="3"> | ||
<Value>StartTimestamp</Value> | ||
</Value> | ||
<Value name="4"> | ||
<Value>EndTimestamp</Value> | ||
</Value> | ||
<Value name="5"> | ||
<Value>HeadRevision</Value> | ||
</Value> | ||
<Value name="6"> | ||
<Value>Status</Value> | ||
</Value> | ||
</Data> | ||
<DataLocation>^SourceContro22B9.DeploymentLogD</DataLocation> | ||
<DefaultData>DeploymentLogDefaultData</DefaultData> | ||
<IdLocation>^SourceContro22B9.DeploymentLogD</IdLocation> | ||
<IndexLocation>^SourceContro22B9.DeploymentLogI</IndexLocation> | ||
<StreamLocation>^SourceContro22B9.DeploymentLogS</StreamLocation> | ||
<Type>%Storage.Persistent</Type> | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -754,6 +754,7 @@ ClassMethod Exists(ByRef pFilename) As %Boolean | |
Return 0 | ||
} | ||
|
||
/// Adds this item to the list of items that are tracked by source control | ||
ClassMethod AddToServerSideSourceControl(InternalName As %String) As %Status | ||
{ | ||
#dim i as %Integer | ||
|
@@ -764,10 +765,6 @@ ClassMethod AddToServerSideSourceControl(InternalName As %String) As %Status | |
continue | ||
} | ||
set @..#Storage@("items", item) = "" | ||
#dim sc as %Status = ..ImportItem(item, 1) | ||
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. Review impact of this on setting timestamps 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 did some code review and testing - I don't think there's going to be any impact on timestamps. In the original diff that added this #125 the intent was to use this to deploy items added in the pull. Since then we've moved that logic back to the pull event handlers, so this is purely a duplicate. Timestamps will get updated because we call ImportItem() separately in every code path where we call AddToServerSideSourceControl(). |
||
if 'sc { | ||
set ec = $$$ADDSC(ec, sc) | ||
} | ||
} | ||
quit ec | ||
} | ||
|
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.
Tighten with try/catch and double check exception handling of callers (make sure they're using the returned %Status)
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.
Improved error handling and confirmed that callers are already checking status.