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

fix: Add a feature flag for token balances and token relationships and enable them #10638

Merged
merged 4 commits into from
Dec 23, 2023

Conversation

Neeharika-Sompalli
Copy link
Member

Reverts this PR #10149

Adds a feature flag tokens.balancesInQueries.enabled to enable/disable the token balances and token relationships information to be returned in queries

@Neeharika-Sompalli Neeharika-Sompalli requested a review from a team December 22, 2023 22:28
@Neeharika-Sompalli Neeharika-Sompalli changed the title Add a feature flag for token balances and token relationships and enable them fix: Add a feature flag for token balances and token relationships and enable them Dec 22, 2023
Copy link

github-actions bot commented Dec 22, 2023

Node: HAPI Test (Token) Results

189 tests  ±0   189 ✔️ ±0   16m 39s ⏱️ +27s
  13 suites ±0       0 💤 ±0 
  13 files   ±0       0 ±0 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 22, 2023

Node: HAPI Test (Crypto) Results

211 tests  ±0   205 ✔️ ±0   18m 15s ⏱️ +34s
  22 suites ±0       6 💤 ±0 
  22 files   ±0       0 ±0 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 22, 2023

Node: Unit Test Results

    2 291 files  ±0      2 291 suites  ±0   54m 51s ⏱️ + 3m 52s
118 617 tests +1  118 582 ✔️ +1  35 💤 ±0  0 ±0 
127 034 runs  +1  126 999 ✔️ +1  35 💤 ±0  0 ±0 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Dec 22, 2023

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (0ca35f5) 62.80% compared to head (fb32cb1) 62.82%.
Report is 1 commits behind head on develop.

Files Patch % Lines
...contract/impl/handlers/ContractGetInfoHandler.java 0.00% 9 Missing ⚠️
.../impl/handlers/CryptoGetAccountBalanceHandler.java 85.00% 0 Missing and 3 partials ⚠️
...app/service/mono/context/primitives/StateView.java 75.00% 0 Missing and 2 partials ⚠️
...no/context/properties/GlobalDynamicProperties.java 50.00% 1 Missing ⚠️
...ken/impl/handlers/CryptoGetAccountInfoHandler.java 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10638      +/-   ##
=============================================
+ Coverage      62.80%   62.82%   +0.01%     
- Complexity     30891    30901      +10     
=============================================
  Files           3376     3376              
  Lines         136345   136413      +68     
  Branches       14207    14218      +11     
=============================================
+ Hits           85634    85698      +64     
+ Misses         47312    47311       -1     
- Partials        3399     3404       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Dec 22, 2023

Node: E2E Test Results

    1 files  ±    0      1 suites  ±0   21m 48s ⏱️ + 21m 48s
311 tests +310  311 ✔️ +311  0 💤 ±0  0  - 1 
333 runs  +332  333 ✔️ +333  0 💤 ±0  0  - 1 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

This pull request removes 1 and adds 311 tests. Note that renamed tests count towards both.
EndToEndTests ‑ initializationError
EndToEndTests ‑ ADDRESS_BOOK_CONTROLCanUpdateADDRESS_BOOK
EndToEndTests ‑ ADDRESS_BOOK_CONTROLCanUpdateNODE_DETAILS
EndToEndTests ‑ AccountsGetPayerRecordsIfSoConfigured
EndToEndTests ‑ Acct57CanMakeSmallChanges
EndToEndTests ‑ Acct57CantMakeLargeChanges
EndToEndTests ‑ AddingSignaturesToExecutedTxFails
EndToEndTests ‑ AddingSignaturesToExecutedTxFailsWithLongTermEnabled
EndToEndTests ‑ AddingSignaturesToNonExistingTxFails
EndToEndTests ‑ AddingSignaturesToNonExistingTxFailsWithLongTermEnabled
EndToEndTests ‑ AddressAliasIdFuzzing
…

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 22, 2023

Node: HAPI Test (Time Consuming) Results

21 tests  ±0     9 ✔️ ±0   24m 51s ⏱️ -8s
  2 suites ±0   12 💤 ±0 
  2 files   ±0     0 ±0 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 22, 2023

Node: HAPI Test (Misc) Results

420 tests  ±0   347 ✔️ ±0   28m 4s ⏱️ -46s
  73 suites ±0     73 💤 ±0 
  73 files   ±0       0 ±0 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 22, 2023

Node: Integration Test Results

280 tests  ±0   280 ✔️ ±0   28m 30s ⏱️ -1s
    5 suites ±0       0 💤 ±0 
    5 files   ±0       0 ±0 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 22, 2023

Node: HAPI Test (Smart Contract) Results

412 tests  ±0   399 ✔️ ±0   49m 52s ⏱️ + 1m 2s
  55 suites ±0     13 💤 ±0 
  55 files   ±0       0 ±0 

Results for commit fb32cb1. ± Comparison against base commit 9f2841b.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

LGTM, tyvm @Neeharika-Sompalli !

@Neeharika-Sompalli Neeharika-Sompalli self-assigned this Dec 23, 2023
@tinker-michaelj
Copy link
Collaborator

Node software changes LGTM 💯 , but here we will end up with a repeated list of tokenRelationships when the feature flag is on and the GetContractInfo query is returning tokenRelationships again.

Instead we need,

            actualInfo = actualInfo.toBuilder()
                    .clearTokenRelationships()
                    .addAllTokenRelationships(actualTokenRels)
                    .build();

to ensure our expectations use only the getAccountDetails response no matter the feature flag.

@Neeharika-Sompalli
Copy link
Member Author

actualInfo = actualInfo.toBuilder()
.clearTokenRelationships()
.addAllTokenRelationships(actualTokenRels)
.build();

Ah yes. Thank you very much @tinker-michaelj 🙏

Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

LGTM!

@Neeharika-Sompalli Neeharika-Sompalli merged commit 7e263db into develop Dec 23, 2023
38 checks passed
@Neeharika-Sompalli Neeharika-Sompalli deleted the add-feature-flag-for-balance-queries branch December 23, 2023 02:45
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.

3 participants