Skip to content

deploy: container history#578

Merged
bobheadxi merged 22 commits into
masterfrom
deploy/293-quick-rollback
Mar 31, 2019
Merged

deploy: container history#578
bobheadxi merged 22 commits into
masterfrom
deploy/293-quick-rollback

Conversation

@seifghazi

@seifghazi seifghazi commented Feb 22, 2019

Copy link
Copy Markdown
Contributor

🎟️ Ticket(s): for #293


👷 Changes

Added containerBucket to manage container history for a deployed projects.

Once a project is successfully deployed, the bucket is updated with the container ID and the git project's most recent commit hash. The commit hash can be later be used to identify successful
builds and rollback to prior version when needed.

Before moving forward we might need to refactor data.go and extract env and container functionality to separate modules.

🔦 Testing Instructions

Explain how to test your changes, if applicable.

@seifghazi seifghazi requested review from a team and bobheadxi February 22, 2019 08:05
@ghost ghost requested review from kimoantiqe and yaoharry February 22, 2019 08:05
Comment thread daemon/inertiad/project/data.go Outdated
Comment thread daemon/inertiad/project/deployment.go

@bobheadxi bobheadxi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is a great start! 😍 left some food for thought - ill have a look again tomorrow

@bobheadxi bobheadxi added the pr: wip in progress but seeking feedback label Feb 22, 2019
@bobheadxi bobheadxi changed the title Deploy/293 quick rollback deploy: container history Feb 23, 2019
Comment thread daemon/inertiad/daemon/up.go Outdated
Comment thread daemon/inertiad/project/deployment.go Outdated
Comment thread daemon/inertiad/daemon/up.go Outdated
Comment thread daemon/inertiad/project/data_test.go
@seifghazi seifghazi added the type: testing ♻️ quality assurance and tests label Mar 6, 2019
Comment thread daemon/inertiad/project/data.go Outdated
Comment thread daemon/inertiad/project/data.go Outdated

@yaoharry yaoharry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me! Make sure to test it.

Comment thread daemon/inertiad/project/data.go
Comment thread daemon/inertiad/daemon/up.go Outdated
Comment thread daemon/inertiad/project/data.go
Comment thread daemon/inertiad/project/data.go Outdated
args args
wantErr bool
}{
{"valid project build", args{"projectB", DeploymentMetadata{"hash", "ID", "status", "time"}, 2}, false},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

might want to add some potential error cases here - what if the project name is empty? etc.

@bobheadxi bobheadxi requested a review from yaoharry March 30, 2019 18:53
@codecov

codecov Bot commented Mar 30, 2019

Copy link
Copy Markdown

Codecov Report

Merging #578 into master will decrease coverage by 0.51%.
The diff coverage is 33.79%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #578     +/-   ##
=========================================
- Coverage   56.33%   55.82%   -0.5%     
=========================================
  Files          68       68             
  Lines        3299     3372     +73     
=========================================
+ Hits         1858     1882     +24     
- Misses       1205     1248     +43     
- Partials      236      242      +6
Impacted Files Coverage Δ
daemon/inertiad/build/builder.go 74.5% <ø> (ø) ⬆️
daemon/inertiad/daemon/up.go 0% <0%> (ø) ⬆️
daemon/inertiad/project/deployment.go 34.13% <0%> (-6.1%) ⬇️
daemon/inertiad/project/data.go 67.26% <64.11%> (-2.07%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bde6916...6008ccd. Read the comment docs.

@bobheadxi bobheadxi added pr: finalized needs review and final approval and removed pr: wip in progress but seeking feedback type: testing ♻️ quality assurance and tests labels Mar 30, 2019
@bobheadxi bobheadxi requested a review from terryz21 March 30, 2019 20:57
Comment thread daemon/inertiad/project/deployment.go Outdated

@bobheadxi bobheadxi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@seifghazi so close! looks like there's just one last thing - a lint error:

https://travis-ci.org/ubclaunchpad/inertia/jobs/513613829#L527

+/home/travis/gopath/src/github.com/ubclaunchpad/inertia/daemon/inertiad/project/data.go:150:1: comment on exported method DeploymentDataManager.AddProjectBuildData should be of the form "AddProjectBuildData ..."

@bobheadxi bobheadxi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's all green! awesome work, and congrats on your first PR 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: finalized needs review and final approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants