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

Rename offline to offchain #1533

Merged
merged 11 commits into from
Nov 2, 2023
Merged

Rename offline to offchain #1533

merged 11 commits into from
Nov 2, 2023

Conversation

Cmdv
Copy link
Contributor

@Cmdv Cmdv commented Oct 11, 2023

Description

this fixes #1532

Checklist

  • Commit sequence broadly makes sense
  • Commits have useful messages
  • New tests are added if needed and existing tests are updated
  • Any changes are noted in the changelog
  • Code is formatted with fourmolu on version 0.10.1.0 (which can be run with scripts/fourmolize.sh)
  • Self-reviewed the diff

Migrations

  • The pr causes a breaking change of type a,b or c
  • If there is a breaking change, the pr includes a database migration and/or a fix process for old values, so that upgrade is possible
  • Resyncing and running the migrations provided will result in the same database semantically

If there is a breaking change, especially a big one, please add a justification here. Please elaborate
more what the migration achieves, what it cannot achieve or why a migration is not possible.

@Cmdv Cmdv requested review from a team as code owners October 11, 2023 14:49
@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from 1bc65d4 to 640d6ab Compare October 11, 2023 14:52
Copy link
Contributor

@kderme kderme left a comment

Choose a reason for hiding this comment

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

A new .sql file is probaby needed since the schema has changed.
Also could you fill the autogenerated check-list of the pr, especially the Migration paragraph which is relevant here?

@sgillespie
Copy link
Contributor

There are some build failures in CI

@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from 640d6ab to 4080175 Compare October 12, 2023 10:33
@Cmdv Cmdv requested review from a team as code owners October 16, 2023 11:28
@sgillespie
Copy link
Contributor

There are still 2 failing tests:

Migration is idempotent:                                 2023-10-17 12:58:24.330 UTC [143] ERROR:  relation "pool_offline_fetch_error" does not exist
2023-10-17 12:58:24.330 UTC [143] STATEMENT:  CREATE INDEX IF NOT EXISTS idx_pool_offline_fetch_error_pmr_id ON pool_offline_fetch_error (pmr_id);
ExitFailure 3

Errors in file: /tmp/migrate-2023-10-17T12:58:24.118194028Z.log

FAIL
      Exception: ExitFailure 1
      Use -p '/Migration is idempotent/' to rerun this test only.

    Insert foreign key missing:                              2023-10-17 12:58:24.350 UTC [147] ERROR:  constraint "unique_off_chain_pool_fetch_error" for table "off_chain_pool_fetch_error" does not exist
2023-10-17 12:58:24.350 UTC [147] STATEMENT:  INSERT INTO off_chain_pool_fetch_error ("pool_id", "fetch_time", "pmr_id", "fetch_error", "retry_count") VALUES (1, '2023-10-17 12:58:24.348073899Z', 1, 'too good', 5) ON CONFLICT ON CONSTRAINT unique_off_chain_pool_fetch_error DO UPDATE SET "pool_id" = EXCLUDED."pool_id" RETURNING id ;
FAIL
      Exception: DbInsertException "OffChainPoolFetchError" (SqlError {sqlState = "42704", sqlExecStatus = FatalError, sqlErrorMsg = "constraint \"unique_off_chain_pool_fetch_error\" for table \"off_chain_pool_fetch_error\" does not exist", sqlErrorDetail = "", sqlErrorHint = ""})
      Use -p '/Insert foreign key missing/' to rerun this test only.

@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from 603ba40 to 610c382 Compare October 17, 2023 13:45
@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from a6b0adc to 248dd5e Compare October 18, 2023 13:45
@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from 248dd5e to 6466fb0 Compare October 18, 2023 15:56
sgillespie
sgillespie previously approved these changes Oct 18, 2023
Copy link
Contributor

@sgillespie sgillespie left a comment

Choose a reason for hiding this comment

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

LGTM!

@Cmdv Cmdv requested a review from kderme October 19, 2023 10:49
schema/migration-2-0029-20231017.sql Outdated Show resolved Hide resolved
@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from 0fc020a to dc5c65a Compare October 20, 2023 13:05
@Cmdv Cmdv requested a review from kderme October 20, 2023 14:41
kderme
kderme previously approved these changes Oct 30, 2023
@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from 6552149 to 29b555e Compare October 31, 2023 08:58
@Cmdv Cmdv force-pushed the 1532-rename-offline-to-offchain branch from 904b7b9 to cc82db1 Compare October 31, 2023 11:12
@Cmdv Cmdv requested a review from kderme October 31, 2023 11:46
@Cmdv Cmdv merged commit 5655e2f into master Nov 2, 2023
18 of 20 checks passed
@iohk-bors iohk-bors bot deleted the 1532-rename-offline-to-offchain branch November 2, 2023 13:35
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.

Rename Offline to OffChain
3 participants