feat!: use lineage instead of path to link states on overview #179

hbollon · 2021-06-30T09:05:33Z

With this PR, the Terraboard overview looks like this:

We can access lineage associated states by clicking on it and we get that view (url: /lineage/b7d2aa5a-812a-2d1f-e5e6-835215928e00):

I also updated search view with lineage column and a associated filter field:

Is that good?

coveralls · 2021-06-30T09:07:34Z

Coverage increased (+0.1%) to 57.556% when pulling f79f764 on hbollon:lineages-states into e44ebce on camptocamp:master.

raphink

A few comments

api/api.go

raphink · 2021-07-14T14:28:56Z

main.go

@@ -183,6 +183,7 @@ func main() {
 	http.HandleFunc(util.GetFullPath("api/version"), getVersion)
 	http.HandleFunc(util.GetFullPath("api/user"), api.GetUser)
 	http.HandleFunc(util.GetFullPath("api/states"), handleWithDB(api.ListStates, database))
+	http.HandleFunc(util.GetFullPath("api/states_lineages"), handleWithDB(api.ListStatesWithLineages, database))


Why not api/lineages directly?

In fact, I'd use api/lineages/<lineage>/states to list states by lineage

This endpoint is a little bit particular and it's not aimed to recover all states of a lineage
His purpose is to return a list of all state's paths with the associated lineage in order to fill the nav select since we need lineage to access to state view (/lineage/<lineage>?versionid=<versionid>)

Hmmmm I think we'll need to talk about that. I'm not sure to understand.

Maybe it's me who explains very badly ^^

This is a new endpoint which return something like this:

[ { "path": "terraform_0.12.28.tfstate", "lineage": "b7d2aa5a-812a-2d1f-e5e6-835215928e00", "version_id": "1234" }, { "path": "terraform_0.13.5.tfstate", "lineage": "b7d2aa5a-812a-2d1f-e5e6-835215928e00", "version_id": "5678" }, ... ]

And I'm using that to populate the state quick access (top-right select element on Terraboard).
I put the path as item label and the whole object as value to be able to build the correct URL when a user select a path to view it.

OK. How about /api/lineages/last?

It doesn't seem very relevant to me since this endpoint returns all the states and not just the last ones.
The drop-down menu that it populates is really intended to list all the path existing in the database (without duplicates however) to allow the user to quickly access a specific state file now that we only list the last by lineage on the overview.

Let's only list lineages and ignore potentially renamed paths.

static/main.html

static/search.html

api/api.go

raphink · 2021-07-15T12:07:34Z

api/api.go

@@ -83,17 +98,17 @@ func ListStateStats(w http.ResponseWriter, r *http.Request, d *db.Database) {
 // GetState provides information on a State
 func GetState(w http.ResponseWriter, r *http.Request, d *db.Database) {
 	w.Header().Set("Access-Control-Allow-Origin", "*")
-	st := util.TrimBasePath(r, "api/state/")
+	lineage := util.TrimBasePath(r, "api/state/")


Shouldn't this be /api/lineage now?

So I don't really know, for me this endpoint as well as api/state/compare/ are always used to perform operations on states, lineages are only one parameter used.
But if you find it more suitable to replace state by lineage, no problem.

I guess that's two different ways of organizing the API. Any preference @cryptobioz ?

raphink · 2021-07-15T12:08:09Z

api/api.go

+	// 		JSONError(w, "Failed to retrieve default version", err)
+	// 		return
+	// 	}
+	// }


Is this code obsolete now?

Yes it took the path in argument.
However I not removed it because I didn't know what this piece of code was for

From what I can tell, this retrieves the default version when it is not passed as an argument.

Should we keep it?
I don't really see cases where the GetState api might not have a versionID and, moreover, we no longer pass the path there, so we would have to retrieve it from the lineage only and I don't know if this is possible (in the possibility that there can be different paths for the same lineage)

raphink · 2021-07-15T12:09:45Z

api/api.go

@@ -124,13 +139,13 @@ func GetStateActivity(w http.ResponseWriter, r *http.Request, d *db.Database) {
 // StateCompare compares two versions ('from' and 'to') of a State
 func StateCompare(w http.ResponseWriter, r *http.Request, d *db.Database) {
 	w.Header().Set("Access-Control-Allow-Origin", "*")
-	st := util.TrimBasePath(r, "api/state/compare/")
+	lineage := util.TrimBasePath(r, "api/state/compare/")


Should this be in api/lineage/compare now, or maybe in api/lineage/<lineage>/compare?

So I don't really know, for me this endpoint as well as api/state/compare/ are always used to perform operations on states, lineages are only one parameter used.
But if you find it more suitable to replace state by lineage, no problem.

raphink · 2021-07-15T12:10:34Z

db/db.go

@@ -131,7 +131,7 @@ func (db *Database) stateS3toDB(sf *statefile.File, path string, versionID strin
 		Version:   version,
 		TFVersion: sf.TerraformVersion.String(),
 		Serial:    int64(sf.Serial),
-		LineageID: sql.NullInt64{Int64: int64(lineage.ID)},
+		LineageID: sql.NullInt64{Int64: int64(lineage.ID), Valid: true},


What does this Valid: true parameter do?

sql.NullInt64 or sql.NullString are types which handle null values for SQL operations.
Since we can't asign nil to int or string, the Valid field is here for that.
Valid: true means that it's not NULL, at false it's a NULL value

db/db.go

raphink · 2021-07-15T12:16:02Z

main.go

@@ -183,6 +183,7 @@ func main() {
 	http.HandleFunc(util.GetFullPath("api/version"), getVersion)
 	http.HandleFunc(util.GetFullPath("api/user"), api.GetUser)
 	http.HandleFunc(util.GetFullPath("api/states"), handleWithDB(api.ListStates, database))
+	http.HandleFunc(util.GetFullPath("api/states_lineages"), handleWithDB(api.ListStatesWithLineages, database))


Hmmmm I think we'll need to talk about that. I'm not sure to understand.

static/lineage.html

mcanevet

I'm not sure that adding an exclamation mark to the PR message is enough. I guess you have to do it on a commit message.

hbollon · 2021-07-15T13:42:41Z

I'm not sure that adding an exclamation mark to the PR message is enough. I guess you have to do it on a commit message.

If I'm not wrong, since @raphink squash and rebase PRs and not merge them it's the PR title which is used on the rebase commit. But I'm not totally sure.

main.go

…t on lineage_id

* rename GetStateActivity functions to GetLineageActivity * change /api/state/activity to /api/lineage/activity * restore dynamic lock status on overview * fix missing version_id attribute in states quick access links * remove lineage display from overview

* refactor(api): update some endpoints path to match new lineage system * feat(server): add CORS middleware to abstract headers modifications * refactor(api): update DefaultVersion function to use lineage

raphink

LGTM

hbollon force-pushed the lineages-states branch from e8f45b0 to 4cbc8ec Compare June 30, 2021 13:52

hbollon mentioned this pull request Jul 1, 2021

feat: migration to Bootstrap 4.1.3 & JQuery 3.3.1 #180

Closed

mcanevet requested a review from raphink July 7, 2021 07:37

mcanevet added this to the v2.0.0-alpha1 milestone Jul 7, 2021

hbollon force-pushed the lineages-states branch 4 times, most recently from c3a05eb to abd2349 Compare July 12, 2021 12:15

raphink suggested changes Jul 14, 2021

View reviewed changes

raphink reviewed Jul 15, 2021

View reviewed changes

raphink changed the title ~~feat: use lineage instead of path to link states on overview~~ feat!: use lineage instead of path to link states on overview Jul 15, 2021

raphink added the breaking label Jul 15, 2021

mcanevet suggested changes Jul 15, 2021

View reviewed changes

hbollon force-pushed the lineages-states branch 2 times, most recently from e81d8c4 to 9fa3851 Compare July 22, 2021 08:01

hbollon mentioned this pull request Jul 22, 2021

feat: front-end redesign to VueJS/Bootstrap 5 & server improvements #188

Merged

raphink reviewed Jul 23, 2021

View reviewed changes

main.go Outdated Show resolved Hide resolved

raphink reviewed Jul 23, 2021

View reviewed changes

main.go Outdated Show resolved Hide resolved

raphink reviewed Jul 23, 2021

View reviewed changes

main.go Outdated Show resolved Hide resolved

hbollon added 8 commits July 23, 2021 15:49

feat: add ListLineageStats api endpoint and model

c2272d4

feat: update Terraboard overview to list by lineage

787469c

feat: add last modified path on lineage overview

620fdb1

feat: adapt total item count for states by lineage view + add distinc…

9ada232

…t on lineage_id

feat: add lineage column on search view with filter

95394ef

feat(lineage): update search view and state explorer to use lineages

25cb5fd

refactor: remove obsolete lineage.html file

f41a383

hbollon force-pushed the lineages-states branch from ff5e4a0 to 45fc569 Compare July 23, 2021 13:54

hbollon requested a review from raphink July 23, 2021 13:55

feat(api): rewrite server using gorilla/mux for routing

f79f764

* refactor(api): update some endpoints path to match new lineage system * feat(server): add CORS middleware to abstract headers modifications * refactor(api): update DefaultVersion function to use lineage

hbollon force-pushed the lineages-states branch from 45fc569 to f79f764 Compare July 23, 2021 13:56

raphink approved these changes Jul 29, 2021

View reviewed changes

raphink merged commit c576d95 into camptocamp:master Jul 29, 2021

This was referenced Jul 29, 2021

chore: release 2.0.0-alpha #187

Merged

chore: release 2.0.0-alpha hbollon/terraboard#3

Closed

chore: release 2.0.0-alpha #190

Merged

chore: release 2.0.0-alpha hbollon/terraboard#4

Open

chore: release 2.0.0-alpha #192

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: use lineage instead of path to link states on overview #179

feat!: use lineage instead of path to link states on overview #179

hbollon commented Jun 30, 2021 •

edited

Loading

coveralls commented Jun 30, 2021 •

edited

Loading

raphink left a comment

raphink Jul 14, 2021

hbollon Jul 15, 2021

raphink Jul 15, 2021

hbollon Jul 15, 2021

raphink Jul 20, 2021

hbollon Jul 20, 2021

raphink Jul 23, 2021

raphink Jul 15, 2021

hbollon Jul 15, 2021

raphink Jul 20, 2021

raphink Jul 15, 2021

hbollon Jul 15, 2021

raphink Jul 20, 2021

hbollon Jul 22, 2021

raphink Jul 15, 2021

hbollon Jul 15, 2021

raphink Jul 15, 2021

hbollon Jul 15, 2021 •

edited

Loading

raphink Jul 15, 2021

mcanevet left a comment

hbollon commented Jul 15, 2021

raphink left a comment

feat!: use lineage instead of path to link states on overview #179

feat!: use lineage instead of path to link states on overview #179

Conversation

hbollon commented Jun 30, 2021 • edited Loading

coveralls commented Jun 30, 2021 • edited Loading

raphink left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hbollon Jul 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcanevet left a comment

Choose a reason for hiding this comment

hbollon commented Jul 15, 2021

raphink left a comment

Choose a reason for hiding this comment

hbollon commented Jun 30, 2021 •

edited

Loading

coveralls commented Jun 30, 2021 •

edited

Loading

hbollon Jul 15, 2021 •

edited

Loading