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

feat: deprecate sunrise tui #825

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

natanasow
Copy link
Collaborator

@natanasow natanasow commented Nov 4, 2024

Description:

Running the hedera start command show the UI in the command line temporarily and then fails with:

─Consensus Node Log─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│[Hedera-Local-Node] INFO (RecoveryState) ⏳ Starting Recovery State...                                                                                                                                                                                                      │
│[Hedera-Local-Node] INFO (RecoveryState) ⏳ Trying to startup again...                                                                                                                                                                                                       │[Hedera-Local-Node] INFO (StartState) ⏳ Detecting network...
│[Hedera-Local-Node] INFO (ConnectionService) Mirror Node GRPC not yet available at: 127.0.0.1:5600. Retrying...                                                                                                                                                             │
│[Hedera-Local-Node] INFO (ConnectionService) Mirror Node GRPC not yet available at: 127.0.0.1:5600. Retrying...                                                                                                                                                             │
│[Hedera-Local-Node] INFO (ConnectionService) Mirror Node GRPC not yet available at: 127.0.0.1:5600. Retrying...
...
[Hedera-Local-Node] INFO (InitState) [✔︎] Needed bootsrap properties were set for this configuration.
[Hedera-Local-Node] INFO (InitState) [✔︎] Needed mirror node properties were set for this configuration.
[Hedera-Local-Node] INFO (StartState) ⏳ Starting Hedera Local Node...
──────────────────────────────────────────────────────────────────────────────

Error on xterm-256color.Setulc:
"\u001b[58::2::%p1%{65536}%/%d::%p1%{256}%/%{255}%&%d::%p1%{255}%&%d%;m"

var v,
 stack = [],
 out = ["\x1b[58::2::"];
(stack.push(v = params[0]),
 v),
(stack.push(v = 65536),
 v),
(v = stack.pop(),
 stack.push(v = (stack.pop() / v) || 0),
 v),
out.push(stack.pop()),
out.push("::"),
(stack.push(v = params[0]),
 v),
(stack.push(v = 256),
 v),
(v = stack.pop(),
 stack.push(v = (stack.pop() / v) || 0),
 v),
(stack.push(v = 255),
 v),
(v = stack.pop(),
 stack.push(v = (stack.pop() & v) || 0),
 v),
out.push(stack.pop()),
out.push("::"),
(stack.push(v = params[0]),
 v),
(stack.push(v = 255),
 v),
(v = stack.pop(),
 stack.push(v = (stack.pop() & v) || 0),
 v),
out.push(stack.pop())}out.push("m");
return out.join("");

The PR completely deprecates the Sunset TUI because it does not reach the desired user experience. Also, it depends on the local user's terminal and might cause unnecessary debugging overhead.

Removing tui makes the detached flag useless, so it's removed as well in this PR.

Related issue(s):

Fixes #820

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@natanasow natanasow changed the title chore: remove sunrise tui feat: deprecate sunrise tui Nov 4, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.72%. Comparing base (1965dbe) to head (ff5204a).

Files with missing lines Patch % Lines
src/services/LoggerService.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #825      +/-   ##
==========================================
+ Coverage   57.59%   63.72%   +6.13%     
==========================================
  Files          30       30              
  Lines        1403     1213     -190     
  Branches      173      145      -28     
==========================================
- Hits          808      773      -35     
+ Misses        574      419     -155     
  Partials       21       21              
Files with missing lines Coverage Δ
src/services/CLIService.ts 5.20% <ø> (+0.20%) ⬆️
src/state/AttachState.ts 100.00% <100.00%> (+23.07%) ⬆️
src/state/StartState.ts 94.44% <ø> (-0.30%) ⬇️
src/services/LoggerService.ts 7.31% <0.00%> (-1.32%) ⬇️

Impacted file tree graph

Copy link

sonarcloud bot commented Nov 4, 2024

@natanasow natanasow marked this pull request as ready for review November 4, 2024 12:47
@natanasow natanasow self-assigned this Nov 4, 2024
@Nana-EC
Copy link
Contributor

Nana-EC commented Nov 4, 2024

@natanasow does this leave a feature gap for users or was this always a nice to have?
We'll want to make sure the UI didn't present any information that isn't available in log outputs or already known to user.

Additionally are we considering a future item to replace it or does this totally remove the terminal interface?

@natanasow
Copy link
Collaborator Author

@Nana-EC This was always nice to have. The UI doesn't present any information that isn't available through logs. We are totally removing the terminal user interface and we will stick to simple CLI tools like some Ethereum local nodes.

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.

hedera start command fails w stack related printout
3 participants