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: Adding debug mode to local node. #465

Merged
merged 4 commits into from
Jan 2, 2024

Conversation

stefan-stefanooov
Copy link
Contributor

Description:
run hedera debug with any valid timestmp returns the record file for that timestamp

Related issue(s):
Fix debug mode and add documentation#269

@stefan-stefanooov stefan-stefanooov added Feature Enhancement Enhancing an existing feature driven by business requirements. Typically backwards compatible. Limechain labels Dec 13, 2023
@stefan-stefanooov stefan-stefanooov added this to the 2.21.0 milestone Dec 13, 2023
@stefan-stefanooov stefan-stefanooov self-assigned this Dec 13, 2023
@stefan-stefanooov stefan-stefanooov linked an issue Dec 13, 2023 that may be closed by this pull request
@stefan-stefanooov stefan-stefanooov force-pushed the 269-fix-debug-mode-and-add-documentation branch from 16a9079 to 2ccfdf1 Compare December 18, 2023 12:33
@stefan-stefanooov stefan-stefanooov force-pushed the 269-fix-debug-mode-and-add-documentation branch from 67a7ffa to e314d8b Compare December 18, 2023 13:04
Copy link
Contributor

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

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

Nice, just a few nits and we'll be ready to go

Comment on lines 21 to 22
local:
deleteAfterProcessing: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this, so we get the default from the mirror-node itself. If we need to enable it, will do so when starting the local-node itself and clean it after starting.

Copy link
Contributor Author

@stefan-stefanooov stefan-stefanooov Dec 19, 2023

Choose a reason for hiding this comment

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

This flag needs to be present in the context of the whole debug workflow.
This is the only way, I think, to double check that the Mirror node is started with "deleting of the record files disabled"

If you want to clear this. We need to start a cleanup procedure on localnode stop

Unfortunately we already execute cleanup state on the start command
SO this means we need to pass the previous step as a parameter of the current. Or something like that...

I think setting this flag on every command is the simplest.

src/services/CLIService.ts Outdated Show resolved Hide resolved
src/state/DebugState.ts Outdated Show resolved Hide resolved
src/state/DebugState.ts Outdated Show resolved Hide resolved
src/state/DebugState.ts Outdated Show resolved Hide resolved
src/utils/config.ts Outdated Show resolved Hide resolved
@stefan-stefanooov stefan-stefanooov force-pushed the 269-fix-debug-mode-and-add-documentation branch 3 times, most recently from ef7083a to d3c6c0a Compare December 19, 2023 10:28
Copy link
Contributor

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

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

few nits

application.hedera.mirror.importer.downloader['local']['deleteAfterProcessing'] = !debugMode

Copy link
Contributor

Choose a reason for hiding this comment

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

Add this to originalNodeProperties.json with key debugNodeProperties and then use it similar to the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK... I did as you asked in the current state of the MR. But I don't get how this solves our problem.

CLIs are not suppose to and can't hold state between command executions.
Because of this, there is no way of knowing (inside the debug command) how was the Local Node started.

And we have a requirement to start it, with the flag we are discussing, set to false.

That's why I created this function checkForDebugMode that checks exactly that. And outputs an error if you try to debug with local node not in debug mode.

We set the flag on InitState but we clean it in CleanUpState

The only way of passing state to it is config file. As far as I can see.

Copy link
Contributor Author

@stefan-stefanooov stefan-stefanooov Dec 19, 2023

Choose a reason for hiding this comment

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

Maybe a good solution would be to combine the two commands:

  • when we start in debug mode. we would start asking the user for timestamp
  • output whatever is in the record file
  • prompt again / or exit

That way we would always be in the context of the start (with debug) command. And we will STOP the execution on exit.
Maybe we could use inquirer or any other lib to achieve that ?

src/utils/config.ts Outdated Show resolved Hide resolved
src/utils/config.ts Show resolved Hide resolved
src/utils/config.ts Outdated Show resolved Hide resolved
@stefan-stefanooov stefan-stefanooov force-pushed the 269-fix-debug-mode-and-add-documentation branch from d3c6c0a to aa7df82 Compare December 19, 2023 14:41
@stefan-stefanooov stefan-stefanooov linked an issue Dec 20, 2023 that may be closed by this pull request
Copy link
Contributor

@Ivo-Yankov Ivo-Yankov left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: Stefan Stefanov <stefan.stefanooov@gmail.com>
Signed-off-by: Stefan Stefanov <stefan.stefanooov@gmail.com>
Signed-off-by: Stefan Stefanov <stefan.stefanooov@gmail.com>
Signed-off-by: Stefan Stefanov <stefan.stefanooov@gmail.com>
@stefan-stefanooov stefan-stefanooov force-pushed the 269-fix-debug-mode-and-add-documentation branch from 7ea281e to 5b1b4c6 Compare January 2, 2024 11:09
Copy link

sonarqubecloud bot commented Jan 2, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

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

LG

@stefan-stefanooov stefan-stefanooov merged commit d85fa6e into main Jan 2, 2024
11 checks passed
@stefan-stefanooov stefan-stefanooov deleted the 269-fix-debug-mode-and-add-documentation branch January 2, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Enhancement Enhancing an existing feature driven by business requirements. Typically backwards compatible. Limechain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debug Mode: Fix the underlying functionality Debug Mode: Adding debug mode and its documentation
4 participants