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.

Sign up for GitHub

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

Jump to bottom

feat!: disable console wallet grpc #5988

Merged
merged 2 commits into from Nov 28, 2023
Merged

feat!: disable console wallet grpc #5988

merged 2 commits into from Nov 28, 2023

Conversation

hansieodendaal
Copy link
Contributor

Description

Disabled the console wallet gRPC via a feature flag in the console wallet build configuration, where gRPC is not part of the default build.
Trying to run with the console wallet with the config option grpc_enabled = true where the gRPC feature is not enabled for the build will result in a runtime error.

Motivation and Context

See #5930

How Has This Been Tested?

Existing unit and cucumber tests pass

What process can a PR reviewer use to test or verify this change?

Code walkthrough

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - The console wallet must be build with the features = ["grpc"] to enable gRPC

@hansieodendaal
Disable Console Wallet gRPC
3e3c046 
Disabled the console wallet gRPC via a feature flag in the build configuration, where
gRPC is not part of the default build.
@hansieodendaal hansieodendaal mentioned this pull request Nov 27, 2023
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 27, 2023 •
edited

Test Results (CI)

1 249 tests   1 249 ✔️  12m 16s ⏱️
     39 suites         0 💤
       1 files           0 ❌

Results for commit 3c99f60.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Nov 27, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 27, 2023 •
edited

Test Results (Integration tests)

31 tests  +31   31 ✔️ +31   15m 32s ⏱️ + 15m 32s
11 suites +11     0 💤 ±  0 
  2 files   +  2     0 ❌ ±  0 

Results for commit 3c99f60. ± Comparison against base commit 89b19f6.

♻️ This comment has been updated with latest results.

SWvheerden
SWvheerden requested changes Nov 28, 2023
View reviewed changes
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

This looks good, but we need to add the grpc option to the docker workflow

SWvheerden
SWvheerden approved these changes Nov 28, 2023
View reviewed changes
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

Will be fixed in another pr

@SWvheerden SWvheerden merged commit 883de17 into tari-project:development Nov 28, 2023
14 checks passed
@hansieodendaal hansieodendaal deleted the ho_disable_wallet_grpc branch November 28, 2023 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SWvheerden SWvheerden SWvheerden approved these changes

Assignees
No one assigned
Labels
P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hansieodendaal @SWvheerden @ghpbot-tari-project