Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: config version alter query #444

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: config version alter query #444

wants to merge 1 commit into from

Conversation

Datron
Copy link
Collaborator

@Datron Datron commented Mar 18, 2025

Problem

When running diesel migration command, I was getting errors as there was a discrepancy between database schemas and models defined in rust

Solution

Updated the query causing the issue and added a make target that can help setup the schema

@Datron Datron requested a review from a team as a code owner March 18, 2025 09:26
DROP COLUMN change_reason;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Datron change is correct ,
can we rename migration file from remove_description -> remove_change_reason

@pratikmishra356 pratikmishra356 self-requested a review March 18, 2025 09:40
file = "src/superposition_schema.rs"
patch_file = "schema.patch"
file = "superposition_schema.rs"
patch_file = "superposition_schema.patch"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we have schema.patch file ?
if its not being used ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm just keeping it consistent incase a diesel migration is run

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, but didnt get the flow over here
saw there is a new make command, didn't get it's use-case

@@ -1,3 +1,3 @@
-- Your SQL goes here
ALTER TABLE public.config_versions
Copy link
Collaborator

@ayushjain17 ayushjain17 Mar 24, 2025

Choose a reason for hiding this comment

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

this query was to be added in workspace_template.sql file as well
db_init.sql was also not updated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

Choose a reason for hiding this comment

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

db_init not updated yet
also, in the db_init file, experiment_status_type does not include DISCARDED type for some reason
it was added there, but apparently in some fix PR it got removed, making it incorrect, can you please also include that as well

@@ -197,3 +197,10 @@ amend-no-edit: COMMIT_FLAGS += --no-edit
amend-no-edit: amend

default: dev-build

schema-file:
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we not add this to .PHONY ? is it returning some usable file ?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants