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.

Sign up for GitHub

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

ci: improve logging for tools build script #1128

Merged
merged 7 commits into from
Nov 30, 2022
Merged

ci: improve logging for tools build script #1128

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cf01f1b
ci: improve logging for tools build script
derberg Nov 24, 2022
c1b3adb
Update tools-object.js
derberg Nov 24, 2022
8858547
updated file
akshatnema Nov 26, 2022
c330825
Merge branch 'master' into fixtoolsrebuildworkflow
derberg Nov 28, 2022
5372705
Update scripts/tools/tools-object.js
derberg Nov 29, 2022
6770186
Merge branch 'master' into fixtoolsrebuildworkflow
akshatnema Nov 30, 2022
36143b2
Update tools-object.js
derberg Nov 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/regenerate-tools.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
- cron: "15 0 * * 1"

jobs:
meetings:
regenerateTools:
if: github.repository == 'asyncapi/website'
env:
GITHUB_TOKEN: ${{ secrets.GH_TOKEN }}
Expand Down
6 changes: 4 additions & 2 deletions scripts/tools/tools-object.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,10 @@ async function convertTools(data) {
}
});
} else {
console.log("Repository: " + tool.repository.html_url)
console.log("Error: " + validate.errors)
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');
Copy link
Member

Choose a reason for hiding this comment

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

Maybe console.error will be better?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

For reference, you can read this documentation - https://api.slack.com/messaging/sending

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

If you want you can add integration with slack. If not, you can do it in another PR :)

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@akshatnema I see you already made changes to console

@akshatnema @magicmatatjahu approve?

}
}
}
Expand Down