[DDW-413] Update support request feature node logs handling #1092
Conversation
|
👍 |
|
@daniloprates please don't forget to implement the logic where we always pack most fresh node logs. We don't want to end up in a situation where we pick just 3 random ones... |
source/main/ipc-api/get-logs.js
Outdated
| @@ -40,13 +23,16 @@ export default () => { | |||
| const logFiles = []; | |||
| if (fs.existsSync(pubLogsFolderPath)) { | |||
| const files = fs.readdirSync(pubLogsFolderPath); | |||
There was a problem hiding this comment.
can you split your loop into two parts: one for Daedalus files and one for node files?
and then, read the node log directory using the regexp ALLOWED_NODE_LOGS?
the reason I ask is because we want to add a symlink to the recent log file, and this symlink should not be included in the log file list.
There was a problem hiding this comment.
also, I don't know if fs.readdirSync will return the file list sorted. Maybe sorting it is a more safe option.
There was a problem hiding this comment.
@CodiePP not sure I understand the need for 2 loops since both will loop through the same files list. What is the symlink file name?
…ature-node-logs-handling
…ature-node-logs-handling
|
@daniloprates @CodiePP this PR is now updated to contain V1 Api integration. The installers produced by the CI are using the tip of the Cardano-sl develop branch for the backend - this means we will be able to see if this new logging setup works or not ;) |
|
🎉
|
|
@daniloprates are you 100% sure these 3 are the latest ones? |
|
@daniloprates also if this from the installer then we should have the launcher log in the list too? |
You are right, it was getting the oldest ones. 0239b06 fixes it:
No other files being generated:
|
|
@daniloprates launcher log should be there too. We need to investigate this. @CodiePP can you assist us here? |
|
@cleverca22 do you have some context on missing launcher log in cardano pub dir? |
|
the launcher log file now has |
|
@cleverca22 are you saying we won't have a separate |
|
its probably a typo, and we just need to fix launcher to write to |
|
@cleverca22 @daniloprates @CodiePP we need to know the exact launcher log file name in order to add it to our logs-packing whitelist. Can we have some more clarity on this one? |
|
we have to adapt the launcher code to reproduce what it was before. will dig it out.. @365andreas will prepare a PR.. |
|
@CodiePP it used to be just |
|
yes, but all the log files that go through |
|
@CodiePP do you think it is sufficient to pack only the latest launcher log? Or should we pack 2 of the latest? Even 3? |
|
I think it can be done the same way as the node log: 3 latest logs. |
|
Thanks @CodiePP! |
|
this is the PR: input-output-hk/cardano-sl#3655 |
|
@CodiePP please notify us here once the Cardano PR has been merged. After that is done we will need to point the cardano revision to the tip of the develop branch in order to generate the installers which will use your code on Cardano side. |
|
@daniloprates I have updated the cardano version used in this PR. After the CI produces the installers we can finally do our end-to-end tests :) |
…ature-node-logs-handling
|
🎉 |
…ature-node-logs-handling
nikolaglumac
deleted the
feature/ddw-413-update-support-request-feature-node-logs-handling
branch
This PR updates the node logs handling for the support request feature.
Todo:
Screenshots:
Review Checklist:
Basics
yarn run test)yarn run dev)yarn run package/ CI builds)yarn run flow:test)yarn run lint)yarn run manage:translationsproduces no changes)yarn run storybook)yarn.lockfile is updatedCode Quality
Testing
After Review:
doneon the Youtrack board