Update the data type of AssetType in trustline tables #268
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
chowbao
reviewed
Jul 17, 2024
amishas157
commented
Jul 19, 2024
internal/transform/trustline.go
Outdated
| @@ -61,7 +61,7 @@ func TransformTrustline(ledgerChange ingest.Change, header xdr.LedgerHeaderHisto | |||
| transformedTrustline := TrustlineOutput{ | |||
| LedgerKey: outputLedgerKey, | |||
| AccountID: outputAccountID, | |||
| AssetType: int32(asset.Type), | |||
| AssetType: asset.Type.String(), | |||
There was a problem hiding this comment.
@chowbao I am seeing values like AssetTypeAssetTypePoolShare in the tables
Should it rather be AssetTypePoolShare ?
There was a problem hiding this comment.
It should match how history_assets does it which I think removes AssetTypeAssetType.
You might need to define a new function to make the change or look for an existing if one exists
This was referenced Jul 22, 2024
amishas157
changed the title
chowbao
reviewed
Jul 22, 2024
internal/transform/trustline.go
Outdated
| @@ -39,12 +39,15 @@ func TransformTrustline(ledgerChange ingest.Change, header xdr.LedgerHeaderHisto | |||
| return TrustlineOutput{}, errors.Wrap(err, fmt.Sprintf("could not create ledger key string for trustline with account %s and asset %s", outputAccountID, asset.ToAsset().StringCanonical())) | |||
| } | |||
|
|
|||
| var assetTypeString string | |||
There was a problem hiding this comment.
nit: I think you don't need to create a new variable and can just reuse assetType because it is already a string variable defined on line 33
In any case this is a non blocking suggestion
chowbao
approved these changes
Jul 22, 2024
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.
PR Checklist
PR Structure
otherwise).
Thoroughness
Release planning
semver, and I've changed the name of the BRANCH to release/_ , feature/_ or patch/* .
What
This PR updates the data type of AssetType for Trustlines tables from integer to string.
Why
This is done to make AssetType consistent across all the schemas as it was mistakenly added as integer earlier.
Known limitations
None. Posted the deploy plan in jira