ci: improve logging for tools build script #1128
Conversation
derberg
requested review from
fmvilas,
mcturco,
akshatnema,
magicmatatjahu and
asyncapi-bot-eve
as code owners
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-1128--asyncapi-website.netlify.app/ |
scripts/tools/tools-object.js
Outdated
| console.log('Invalid .asyncapi-tool file.'); | ||
| console.log(`Located in: ${tool.repository.html_url}`); | ||
| console.log('Validation errors:', JSON.stringify(validate.errors, null, 2)); | ||
| console.log('Not failing, dropping errors for further investigation'); |
There was a problem hiding this comment.
Maybe console.error will be better?
There was a problem hiding this comment.
Yeah, console.error is better but I think what we discussed in the previous meetings that we should notify about each error in the Slack notification channel. That functionality is left here. Will soon add it to the upcoming PR.
There was a problem hiding this comment.
yeah, getting stuff to slack will be tricky, as normal flow will not work. We will need to use GitHub Action core library to set output for GH Action workflow, so we can react and drop message to slack. I can add it in this PR if ya want
There was a problem hiding this comment.
But if you want to use Slack notifications, I would suggest you to use REST API of Slack (if exists 😅) as the log messages are present in the script files and making a API call from that script file to create notifications is lot more easier and reliable instead of using Github Actions.
There was a problem hiding this comment.
For reference, you can read this documentation - https://api.slack.com/messaging/sending
There was a problem hiding this comment.
but GitHub action that we would plug in is using Slack REST API. What is the point of writing own logic?
we can grab log message from script, output to GH Action, and push it to slack, formatted as we want -> https://github.com/asyncapi/community/blob/master/.github/workflows/notify-tsc-members-mention.yml#L82-L86
There was a problem hiding this comment.
If you want you can add integration with slack. If not, you can do it in another PR :)
There was a problem hiding this comment.
now that I think about it, better would be to add integration later after tools view is released because we will get slack notifications now that will annoy us (there is already one file that is invalid in one repo). I'll add issue so we don't forget
There was a problem hiding this comment.
@akshatnema I see you already made changes to console
@akshatnema @magicmatatjahu approve?
| console.log("Repository: " + tool.repository.html_url) | ||
| console.log("Error: " + validate.errors) | ||
| console.error('Invalid .asyncapi-tool file.'); | ||
| console.error(`Located in: ${tool.html_url}`); |
There was a problem hiding this comment.
Checking code, it should be probably:
| console.error(`Located in: ${tool.html_url}`); | |
| console.error(`Located in: ${tool.repository.html_url}`); |
or am I wrong?
There was a problem hiding this comment.
Yeah, from my perspective, certain users are making .asyncapi-tool files inside any subdirectories. If we want to locate the wrong file, it may occur that we get into trouble finding the file inside the repository sub-directories. Hence, using tool.html_url is the best link to directly hop into the file in the respective repository to see the errors, instead using the repository's URL. WDYT @magicmatatjahu @derberg ?
There was a problem hiding this comment.
@akshatnema you are right, tool.html_url provides direct URL to the file
@magicmatatjahu changed
folks, please approve
There was a problem hiding this comment.
Sorry, I was looking at the code and we used a different field. Approved :)
Co-authored-by: Maciej Urbańczyk <[email protected]>
|
/rtm |
related to #940 (comment)