-
Notifications
You must be signed in to change notification settings - Fork 41
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
Allow customisation of node config #1248
Conversation
9d5a26a
to
07ea86e
Compare
I can't really do anything because: |
@monikasmolarek , got it ,on it |
37a4a36
to
da5355a
Compare
Tested only on Windows this time:
|
@@ -3,6 +3,6 @@ module.exports = { | |||
preset: 'ts-jest', | |||
runner: '@jest-runner/electron/main', | |||
testEnvironment: 'node', | |||
testRegex: '(/tests/.*|(\\.|/)(test|spec))\\.ts$', | |||
testMatch: ['**/?(*.)+(spec|test).ts'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update, to be able to run tests locally for a file or just 1 test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yarn test ./tests/some.spec.ts
was working well previously, but this seems working too :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest nodeConfig - will run 1 file
jest -t 'name of the test' will run only 1 specific test
to simplify the test creation process.
59f00ff
to
e1eb10d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Ok, I checked with the latest release exactly the same steps- after switching the network, the node got up with just 1 click on the "restart node" button on the Network screen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
❗ Sounds pretty critical. After switching the network User should not click any "Restart" buttons.
Yeah... go-spacemesh and his API a bit hanging because of the big batch size on a slow GPU :(
@monikasmolarek please open an issue for it 🙏 p.s. @maparr please in case of any further changes — push separate commits to make it easier to review. |
all resolved.
only this restart network cannot reproduce will be testing on Windows Cloud PC |
Hi @monikasmolarek , did the check on Windows, ( virtual ) , with not the best configuration. As result, yes, I'm getting this issue, but the main point on restart, need to wait for a long time 1 minute at least, for the not-best PC, till the node will start again. My configuration has only a CPU. But for both tries, the node is starting after switching networks. For Mac m1 max, it is taking 10 secs ( or even 1 ), for CPU Intel or what we have for the cloud, it takes longer. I think we need to create and issue for this case ( for Windows or PC with not perfect configuration) and create maybe some fix after the discussion, like, please wait for, and not show your node is down if it is running. But it definitely not an issue with this PR also, I think, because of tons of logs, #1283 is related not only macOS , but to Windows first. issue with timestamps resolved and will be available for review when this PR will be merged - #1326 cc: @brusherru |
@@ -459,7 +459,7 @@ const Node = ({ history, location }: Props) => { | |||
<Address | |||
key="smesherCoinbase" | |||
type={AddressType.ACCOUNT} | |||
address={coinbase} | |||
address={coinbase || ''} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because of a fast new flow , we're supporting smeshing from different folders from different configs ( right now each network may have its own smeshing options) , during node restart we're updating and smeshing process and till we're waiting for updates from the node, for 1 ms , we can get new update from the node where the coinbase not updated yet. It is just an issue with the update and react renderer. The user won't see an empty address
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems after https://github.com/spacemeshos/smapp/pull/1334/files#diff-2f272b881ae8ff902e02cbd366f81c899b63396572563819304c10f4ebd5e769R547 merge It, my changes not needed. At least will be rewritten.
It closes #1019
Additionally: resolving an issue with stale genesisID for managers, because of spawn managers, currently in develop, we're passing the genesisID only once and after it, reuse the genesisID and only change it on the wallet change or app restart.
UPD How to test
2 case
1 . set up smeshing
6 . switch back to the first network and compare.
Screen.Recording.2023-06-22.at.09.49.10.mov