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

Allow angle brackets in DOI validation (#513) #514

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

rhigman
Copy link
Member

@rhigman rhigman commented Sep 20, 2023

Fixes #513.

@rhigman rhigman requested a review from ja573 September 20, 2023 14:55
@rhigman
Copy link
Member Author

rhigman commented Sep 20, 2023

@ja573 note the run-migrations failure - the migration was successful locally. I think this is because (as previously discussed) Diesel interprets the migrations as being in the wrong order (strict alphanumeric):

rh@obp-rh:~/code/thoth/thoth-api$ diesel migration list
Migrations:
  [X] 0.0.0_diesel_initial_setup
  [X] 0.1.0
  [X] 0.10.0
  [X] 0.11.3
  [X] 0.11.7
  [X] 0.2.0
  [X] 0.2.11
  [X] 0.3.0
  [X] 0.3.5
  [X] 0.4.1
  [X] 0.4.2
  [X] 0.4.5
  [X] 0.5.0
  [X] 0.6.0
  [X] 0.7.0
  [X] 0.7.2
  [X] 0.8.0
  [X] 0.8.11
  [X] 0.8.3
  [X] 0.8.5
  [X] 0.8.8
  [X] 0.8.9
  [X] 0.9.0
  [X] 0.9.16
  [X] 0.9.2
  [X] 0.9.6

If run-migrations starts with a clean database and tries to run 0.0.0, then 0.1.0, then 0.10.0, etc, the 0.11.7 migration will be the first one to fail as it is the first one to try and alter a table which wasn't created in 0.1.0 (reference). Our local machines' databases already have all of the pre-0.11.7 migrations, so just that one is run and it's successful.

@ja573
Copy link
Member

ja573 commented Sep 21, 2023

@ja573 note the run-migrations failure - the migration was successful locally. I think this is because (as previously discussed) Diesel interprets the migrations as being in the wrong order (strict alphanumeric):

rh@obp-rh:~/code/thoth/thoth-api$ diesel migration list
Migrations:
  [X] 0.0.0_diesel_initial_setup
  [X] 0.1.0
  [X] 0.10.0
  [X] 0.11.3
  [X] 0.11.7
  [X] 0.2.0
  [X] 0.2.11
  [X] 0.3.0
  [X] 0.3.5
  [X] 0.4.1
  [X] 0.4.2
  [X] 0.4.5
  [X] 0.5.0
  [X] 0.6.0
  [X] 0.7.0
  [X] 0.7.2
  [X] 0.8.0
  [X] 0.8.11
  [X] 0.8.3
  [X] 0.8.5
  [X] 0.8.8
  [X] 0.8.9
  [X] 0.9.0
  [X] 0.9.16
  [X] 0.9.2
  [X] 0.9.6

If run-migrations starts with a clean database and tries to run 0.0.0, then 0.1.0, then 0.10.0, etc, the 0.11.7 migration will be the first one to fail as it is the first one to try and alter a table which wasn't created in 0.1.0 (reference). Our local machines' databases already have all of the pre-0.11.7 migrations, so just that one is run and it's successful.

Ok, in that case just change the name of the migration to something that will ensure it runs at the end. When we release v1 I'm planning on combining all migrations into a single one and start anew with migration names that follow the right structure anyway, so it's fine to change the convention now to avoid current problems

ja573
ja573 previously approved these changes Sep 21, 2023
…x failing check (Diesel orders migrations strictly alphanumerically)
@rhigman rhigman merged commit f65bc0c into develop Sep 21, 2023
12 checks passed
@rhigman rhigman deleted the feature/513_further_doi_regex branch September 21, 2023 11:13
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.

None yet

2 participants