Refeshes all tables in Hub DB #301

PuneetPunamiya · 2021-07-30T11:51:46Z

This patch collects the existing data from db deletes
the existing tables, recreates the tables back and
inserts the data in order to update the db and makes it
compatible with gorm 2.0

Signed-off-by: Puneet Punamiya ppunamiy@redhat.com

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Run API Unit Tests, Lint Checks, API Design, Golden Files with make api-check
Run UI Unit Tests, Lint Checks with make ui-check
Commit messages follow commit message best practices

See the contribution guide for more details.

vinamra28

/lgtm

pratap0007 · 2021-08-02T07:32:41Z

/lgtm

sm43 · 2021-08-02T07:45:31Z

api/pkg/db/migration/migration.go

 			updateResourcesCategoryTable(log),
-			removeExistingCategories(log),


why are we removing migrations?

don't we need to run them after refreshing the db?

refreshAllTables this migration has to be run before all newly added migration, after that the remaning will be executed properly

nope.... refresh_all_tables migration will do the job for us

did you test this on a cluster?

sm43 · 2021-08-02T07:47:59Z

api/pkg/db/migration/202107291608_refresh_all_tables.go

+			}
+
+			txn := db.Begin()
+			if err := db.Migrator().DropTable(&model.Category{},


we are starting a txn but not using them while executing queries then how it would be helpful 😸

https://gorm.io/docs/transactions.html#Transactions-by-manual

txn := db.Begin() if err:= refreshDb(txn, log); err!=nil{ log error txn.rollback() } txn.commit

and use txn passed for all queries

api/pkg/db/migration/202107291608_refresh_all_tables.go

linux-foundation-easycla · 2021-08-02T09:15:24Z

The committers are authorized under a signed CLA.

✅ Puneet Punamiya (7bc7f5a)

vinamra28 · 2021-08-04T07:09:20Z

/lgtm

PuneetPunamiya · 2021-08-04T07:46:05Z

/test pull-tekton-hub-build-tests

sm43 · 2021-08-04T07:44:01Z

api/pkg/db/migration/202107131802_remove_existing_categories.go

@@ -28,7 +27,7 @@ func removeExistingCategories(log *log.Logger) *gormigrate.Migration {
 	return &gormigrate.Migration{
 		ID: "202107131802_remove_existing_categories",
 		Migrate: func(db *gorm.DB) error {
-			if err := db.Unscoped().Where("name IN ?", []string{"Deploy", "Editor", "Language", "Notification", "Others", "Test Framework"}).Delete(&model.Category{}).Error; err != nil {
+			if err := db.Exec("DELETE FROM categories").Error; err != nil {


I think we can remove this migration now. as we are updating the config refresh api to handle deletion
??

But before running the api we need to fix the categories as well right ??

to avoid deletion of all records which are not necessarily to be deleted we are updating the config refresh api
so refreshall migration will add those back in db
and when api server is started the new will get added the ones removed from config will get deleted

Yeah agreed on that but ones removed from config which are supposed to be deleted needs to be handled at the api level itself then right

sm43 · 2021-08-04T07:45:21Z

api/pkg/db/migration/202107291608_refresh_all_tables.go

+		return err
+	}
+
+	if len(catalog) > 0 {


just curious when we fetch records and insert as it is ... do the creation timestamp changes ? or remains same as before

I think it should remain the same since we are inserting the data which we have collected from the db itself and also the id for all is not changed so I think it should remain the same

sm43 · 2021-08-04T07:50:50Z

api/test/fixtures/sync_jobs.yaml

@@ -15,11 +15,12 @@
 # NOTE:
 # This record is added to make sure the sync_jobs table state remains same
 # for every catalog refresh test. not adding this will fails the test due to the
-# sequence which will be out of sync and have to manually bring it back in sync. 
+# sequence which will be out of sync and have to manually bring it back in sync.
 # Error: ERROR: duplicate key value violates unique constraint "sync_jobs_pkey"

 - id: 1


jfyi the test didn't failed due to this record because we don't use this record in test.
see the note above :)

api/pkg/db/migration/202107291608_refresh_all_tables.go

- This patch collects the existing data from db deletes the existing tables, recreates the tables back and inserts the data in order to update the db and makes it compatible with `gorm 2.0` Signed-off-by: Puneet Punamiya <ppunamiy@redhat.com>

vinamra28

/lgtm

piyush-garg · 2021-08-05T06:10:37Z

/lgtm

sm43 · 2021-08-05T06:17:35Z

/approve
/meow

tekton-robot · 2021-08-05T06:17:36Z

@sm43:

In response to this:

/approve
/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tekton-robot · 2021-08-05T06:17:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sm43, vinamra28

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sm43]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tekton-robot requested review from sm43 and sthaha July 30, 2021 11:51

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 30, 2021

PuneetPunamiya force-pushed the fix-gorm branch from b3b2411 to eda6cc4 Compare July 30, 2021 12:13

vinamra28 approved these changes Aug 2, 2021

View reviewed changes

tekton-robot assigned vinamra28 Aug 2, 2021

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2021

tekton-robot assigned pratap0007 Aug 2, 2021

sm43 reviewed Aug 2, 2021

View reviewed changes

api/pkg/db/migration/202107291608_refresh_all_tables.go Outdated Show resolved Hide resolved

sm43 reviewed Aug 2, 2021

View reviewed changes

api/pkg/db/migration/202107291608_refresh_all_tables.go Outdated Show resolved Hide resolved

vinamra28 force-pushed the fix-gorm branch from eda6cc4 to 51e5b00 Compare August 2, 2021 09:15

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2021

vinamra28 force-pushed the fix-gorm branch 3 times, most recently from 4454e10 to 520ddae Compare August 2, 2021 09:24

PuneetPunamiya force-pushed the fix-gorm branch 6 times, most recently from 7ba466d to 137fe44 Compare August 4, 2021 07:06

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2021

sm43 requested changes Aug 4, 2021

View reviewed changes

sm43 reviewed Aug 4, 2021

View reviewed changes

api/pkg/db/migration/202107291608_refresh_all_tables.go Show resolved Hide resolved

PuneetPunamiya force-pushed the fix-gorm branch from 137fe44 to 264e7da Compare August 4, 2021 08:24

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2021

PuneetPunamiya force-pushed the fix-gorm branch from 264e7da to e7e549f Compare August 4, 2021 10:16

sm43 reviewed Aug 5, 2021

View reviewed changes

api/pkg/db/migration/202107291608_refresh_all_tables.go Show resolved Hide resolved

PuneetPunamiya force-pushed the fix-gorm branch 2 times, most recently from 1b2aa3f to af3b807 Compare August 5, 2021 04:27

Refreshes all tables in Hub DB

ca61af1

- This patch collects the existing data from db deletes the existing tables, recreates the tables back and inserts the data in order to update the db and makes it compatible with `gorm 2.0` Signed-off-by: Puneet Punamiya <ppunamiy@redhat.com>

PuneetPunamiya force-pushed the fix-gorm branch from af3b807 to ca61af1 Compare August 5, 2021 04:28

vinamra28 reviewed Aug 5, 2021

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2021

tekton-robot assigned piyush-garg Aug 5, 2021

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 5, 2021

tekton-robot merged commit 9488f5d into tektoncd:main Aug 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refeshes all tables in Hub DB #301

Refeshes all tables in Hub DB #301

PuneetPunamiya commented Jul 30, 2021

vinamra28 left a comment

pratap0007 commented Aug 2, 2021

sm43 Aug 2, 2021

sm43 Aug 2, 2021

sm43 Aug 2, 2021

vinamra28 Aug 2, 2021

sm43 Aug 2, 2021

vinamra28 Aug 2, 2021

sm43 Aug 2, 2021

sm43 Aug 2, 2021

sm43 Aug 2, 2021

linux-foundation-easycla bot commented Aug 2, 2021 •

edited

Loading

vinamra28 commented Aug 4, 2021

PuneetPunamiya commented Aug 4, 2021

sm43 Aug 4, 2021

PuneetPunamiya Aug 4, 2021

sm43 Aug 4, 2021

PuneetPunamiya Aug 4, 2021

sm43 Aug 4, 2021

PuneetPunamiya Aug 4, 2021

sm43 Aug 4, 2021

vinamra28 left a comment

piyush-garg commented Aug 5, 2021

sm43 commented Aug 5, 2021

tekton-robot commented Aug 5, 2021

tekton-robot commented Aug 5, 2021

		updateResourcesCategoryTable(log),
		removeExistingCategories(log),

Refeshes all tables in Hub DB #301

Refeshes all tables in Hub DB #301

Conversation

PuneetPunamiya commented Jul 30, 2021

Changes

Submitter Checklist

vinamra28 left a comment

Choose a reason for hiding this comment

pratap0007 commented Aug 2, 2021

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

linux-foundation-easycla bot commented Aug 2, 2021 • edited Loading

vinamra28 commented Aug 4, 2021

PuneetPunamiya commented Aug 4, 2021

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

vinamra28 left a comment

Choose a reason for hiding this comment

piyush-garg commented Aug 5, 2021

sm43 commented Aug 5, 2021

tekton-robot commented Aug 5, 2021

tekton-robot commented Aug 5, 2021

linux-foundation-easycla bot commented Aug 2, 2021 •

edited

Loading