Skip to content
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

deploy: container history #578

Merged
merged 22 commits into from
Mar 31, 2019
Merged

deploy: container history #578

merged 22 commits into from
Mar 31, 2019

Conversation

seifghazi
Copy link
Contributor

@seifghazi seifghazi commented Feb 22, 2019

🎟️ 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 bobheadxi and a team February 22, 2019 08:05
@ghost ghost requested review from kimoantiqe and yaoharry February 22, 2019 08:05
Copy link
Member

@bobheadxi bobheadxi left a comment

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
daemon/inertiad/project/deployment.go Outdated Show resolved Hide resolved
daemon/inertiad/daemon/up.go Outdated Show resolved Hide resolved
@seifghazi seifghazi added the type: testing ♻️ quality assurance and tests label Mar 6, 2019
Copy link
Member

@yaoharry yaoharry left a comment

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.

daemon/inertiad/daemon/up.go Outdated Show resolved Hide resolved
daemon/inertiad/project/data.go Show resolved Hide resolved
daemon/inertiad/project/data.go Outdated Show resolved Hide resolved
args args
wantErr bool
}{
{"valid project build", args{"projectB", DeploymentMetadata{"hash", "ID", "status", "time"}, 2}, false},
Copy link
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
Copy link

codecov bot commented Mar 30, 2019

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
Copy link
Member

@bobheadxi bobheadxi left a comment

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 ..."

Copy link
Member

@bobheadxi bobheadxi left a comment

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