-
Notifications
You must be signed in to change notification settings - Fork 166
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
feat: add plan db scheme #164
Conversation
@hbollon don't we have the |
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.
Where is the field corresponding to the bucket? Is that the field Source
?
No it is not in the model currently but it's a good idea I'll add a field for that
The field representing the associated bucket is VersionID. The Source field on the other hand, if I am referring to TerraVerge, is a trace of the system at the origin of the Terraform plan (like the Gitlab pipeline) |
Okay, that's great! Regarding the bucket field, would it make sense to name it like |
I thought the same thing but the structure for the bucket is called Version in the Go code (defined in |
You're totally right, I didn't read the whole file... Then I'm okay with what you did :) @raphink, could you give us some explanation regarding how you store the S3 bucket information? |
Another notable thing that I would have wanted you to validate is the use of the go-gorm/datatypes library used to be able to use a jsonb field for store the raw plan output. |
I guess it's okay for me :) |
I just thought that the CreatedAt and UpdatedAt fields being managed automatically by Gorm during the insertion, we will not have the real date of creation of the plan. Will it be useful to add a field to store it? |
Well, if everything is automated, you will have the right creation date. We should keep these fields and let Gorm manage them. |
* fix CiURL field json tag
I think my understanding of the Version structure was wrong, I thought it represented the Bucket but in reality this structure represents the version of a State. I don't understand the comment of this object by the way, "Version is an S3 bucket version", wouldn't it be "a State Version" instead? If I'm not mistaken, with Terraform we don't version Buckets but States files right? |
Indeed, I guess the link between state and plane should be done on lineage, which is, from my understanding, the unique ID that identifies a workspace. lineage+serial identifies a version of the state. lineage+"execution time" should identify a plan. |
Okay so I removed it. Now this PR should be mergeable if it's all good for you 😄 |
* added varchar(10) constraint to State TFVersion * replaced CreatedAt/UpdatedAt
Let's merge this for now so @hbollon can start working on gorm.v2. |
I used the db scheme available in the TerraVerge repository to which I added a foreign key to the associated bucket (I thought it would be useful if we add support for several simultaneous bucket) and the CreatedAt / UpdatedAt columns
Here is the table that it creates (visualized from pgadmin):