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

fix: Add migration for changes in TFOD and CVAT template #756

Merged
merged 9 commits into from
Nov 30, 2020

Conversation

savan77
Copy link
Contributor

@savan77 savan77 commented Nov 30, 2020

What this PR does:

Which issue(s) this PR fixes:

Fixes onepanelio/core#

Special notes for your reviewer:

Checklist

Please check if applies

  • I have added/updated relevant unit tests
  • I have added/updated relevant documentation

Required

  • I accept to release these changes under the Apache 2.0 License

mountPath: /etc/onepanel
readOnly: true
- name: cvat-ui
image: onepanel/cvat-ui:0.14.0_cvat.1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look like the latest template

@rushtehrani
Copy link
Contributor

The migrations also need to be added to db/go/db.go

@rushtehrani rushtehrani changed the title update: add migration for changes in TFOD and CVAT template fix: Add migration for changes in TFOD and CVAT template Nov 30, 2020
@rushtehrani
Copy link
Contributor

Can you update the TFOD template based on the last migration template? It'll make it easier to diff.

@savan77
Copy link
Contributor Author

savan77 commented Nov 30, 2020

@rushtehrani can you check now?


//Down20201130130433 do nothing
func Down20201130130433(tx *sql.Tx) error {
// This code is executed when the migration is rolled back.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should update to the previous template:

return updateWorkflowTemplateManifest(
		"20201115134934_tfod.yaml",
		tfodWorkflowTemplateName,
		map[string]string{
			"used-by": "cvat",
		},
	)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to confirm, inside Down20201130130433?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, only in down migration. Looks like you changed up migration to use the old template too?

func Up20201130130433(tx *sql.Tx) error {
// This code is executed when the migration is applied.
return updateWorkflowTemplateManifest(
"20201115134934_tfod.yaml",
Copy link
Contributor

@rushtehrani rushtehrani Nov 30, 2020

Choose a reason for hiding this comment

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

This looks incorrect now, should be: 20201130130433_tfod.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, updating now.

@rushtehrani rushtehrani added this to the v0.16.0 milestone Nov 30, 2020
@rushtehrani rushtehrani merged commit 762abe8 into onepanelio:master Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants