Accounting Style Renaming: proper manual migration #2263
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I originally added a 'backward-compatible' function step to the deserialization function of value of type 'AccountingStyle'. The problem is that it only concerns objects that are being deserialized. Any 'AccountingStyle' value that is queried will inevitably fail because columns in an existing database would still be using the old serialization format. As a consequence, fetching all addresses of an existing sequential wallet returns an empty set, regardless of what is in the database.
KtorZ
added
the
RESOLVING ISSUE
KtorZ
force-pushed
the
KtorZ/ADP-523/accounting-style-renaming
branch
from
0a6c36c to
a74532d
Compare
piotr-iohk
approved these changes
Oct 22, 2020
| @@ -262,6 +263,10 @@ spec = do | |||
| , minBound | |||
| ) | |||
|
|
|||
| it "'migrate' db with old text serialization for 'AccountingStyle'" $ | |||
| testMigrationAccountingStyle @ShelleyKey | |||
| "shelleyAccountingStyle-v2020-10-13.sqlite" | |||
|
Actually creating new wallet here results in: (Seems that tests complain about that also) |
KtorZ
force-pushed
the
KtorZ/ADP-523/accounting-style-renaming
branch
from
a74532d to
7257a4c
Compare
|
@piotr-iohk yes, and same for random wallet, see the last change here: https://github.com/input-output-hk/cardano-wallet/compare/a74532de74f0c0e25e3b45ba73b89d1e3eaf0a7f..7257a4cee4d363f67671adfb11698d7049a5d115 We need to control that the table exists first 😅 ... |
|
bors merge |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 22, 2020
2263: Accounting Style Renaming: proper manual migration r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> ADP-523 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 016f60c 📍 **reproduce deserialization issue with regards to account style change** I originally added a 'backward-compatible' function step to the deserialization function of value of type 'AccountingStyle'. The problem is that it only concerns objects that are being deserialized. Any 'AccountingStyle' value that is queried will inevitably fail because columns in an existing database would still be using the old serialization format. As a consequence, fetching all addresses of an existing sequential wallet returns an empty set, regardless of what is in the database. - 0a6c36c 📍 **use a proper manual database migration for renaming AccountingStyle instead of the hacky in-place adjustment in the deserializer.** # Comments <!-- Additional comments or screenshots to attach if any --> Whoops... <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]>
KtorZ
force-pushed
the
KtorZ/ADP-523/accounting-style-renaming
branch
from
7257a4c to
41c092b
Compare
|
bors merge |
|
Canceled. |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 22, 2020
2263: Accounting Style Renaming: proper manual migration r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> ADP-523 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 016f60c 📍 **reproduce deserialization issue with regards to account style change** I originally added a 'backward-compatible' function step to the deserialization function of value of type 'AccountingStyle'. The problem is that it only concerns objects that are being deserialized. Any 'AccountingStyle' value that is queried will inevitably fail because columns in an existing database would still be using the old serialization format. As a consequence, fetching all addresses of an existing sequential wallet returns an empty set, regardless of what is in the database. - 0a6c36c 📍 **use a proper manual database migration for renaming AccountingStyle instead of the hacky in-place adjustment in the deserializer.** # Comments <!-- Additional comments or screenshots to attach if any --> Whoops... <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]>
instead of the hacky in-place adjustment in the deserializer.
KtorZ
force-pushed
the
KtorZ/ADP-523/accounting-style-renaming
branch
from
41c092b to
8629a66
Compare
|
Canceled. |
|
bors merge |
|
bors r- |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 22, 2020
2229: add verification key endpoint r=KtorZ a=paweljakubas # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> https://jira.iohk.io/browse/ADP-474 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have added endpoint plus basic types and instances - [x] I have extended swagger - [x] I have updated core unit tests - [x] I have added impl and switched on impl in Shelley - [x] I have extended Api.Link and core-integration's DSL - [x] I have added illustrative integration test - [x] I have added golden tests for explicit mnemonic and picked indices based on cardano-addresses # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2263: Accounting Style Renaming: proper manual migration r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> ADP-523 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 016f60c 📍 **reproduce deserialization issue with regards to account style change** I originally added a 'backward-compatible' function step to the deserialization function of value of type 'AccountingStyle'. The problem is that it only concerns objects that are being deserialized. Any 'AccountingStyle' value that is queried will inevitably fail because columns in an existing database would still be using the old serialization format. As a consequence, fetching all addresses of an existing sequential wallet returns an empty set, regardless of what is in the database. - 0a6c36c 📍 **use a proper manual database migration for renaming AccountingStyle instead of the hacky in-place adjustment in the deserializer.** # Comments <!-- Additional comments or screenshots to attach if any --> Whoops... <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2264: Fix error message on bad query param filtering addresses in Jormungandr tests r=piotr-iohk a=piotr-iohk # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> # Overview - 67ef8fb Fix error message on bad query param filtering addresses in Jormungandr tests # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]>
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Canceled. |
|
bors merge |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 22, 2020
2229: add verification key endpoint r=KtorZ a=paweljakubas # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> https://jira.iohk.io/browse/ADP-474 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have added endpoint plus basic types and instances - [x] I have extended swagger - [x] I have updated core unit tests - [x] I have added impl and switched on impl in Shelley - [x] I have extended Api.Link and core-integration's DSL - [x] I have added illustrative integration test - [x] I have added golden tests for explicit mnemonic and picked indices based on cardano-addresses # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2263: Accounting Style Renaming: proper manual migration r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> ADP-523 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 016f60c 📍 **reproduce deserialization issue with regards to account style change** I originally added a 'backward-compatible' function step to the deserialization function of value of type 'AccountingStyle'. The problem is that it only concerns objects that are being deserialized. Any 'AccountingStyle' value that is queried will inevitably fail because columns in an existing database would still be using the old serialization format. As a consequence, fetching all addresses of an existing sequential wallet returns an empty set, regardless of what is in the database. - 0a6c36c 📍 **use a proper manual database migration for renaming AccountingStyle instead of the hacky in-place adjustment in the deserializer.** # Comments <!-- Additional comments or screenshots to attach if any --> Whoops... <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> 2264: Fix error message on bad query param filtering addresses in Jormungandr tests r=KtorZ a=piotr-iohk # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> # Overview - 67ef8fb Fix error message on bad query param filtering addresses in Jormungandr tests # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]> Co-authored-by: IOHK <[email protected]> Co-authored-by: Piotr Stachyra <[email protected]>
|
This PR was included in a batch that timed out, it will be automatically retried |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
ADP-523
Overview
016f60c
📍 reproduce deserialization issue with regards to account style change
I originally added a 'backward-compatible' function step to the deserialization function of value of type 'AccountingStyle'. The problem is that it only concerns objects that are being deserialized. Any 'AccountingStyle' value that is queried will inevitably fail because columns in an existing database would still be using the old serialization format. As a consequence, fetching all addresses of an existing sequential wallet returns an empty set, regardless of what is in the database.
0a6c36c
📍 use a proper manual database migration for renaming AccountingStyle instead of the hacky in-place adjustment in the deserializer.
Comments
Whoops...