-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fix #153: Prevent stdout cutoff in Azure CLI versions #154
Conversation
src/main.ts
Outdated
await exec.exec("curl", ["--location", "-s", "https://mcr.microsoft.com/v2/azure-cli/tags/list"], execOptions) | ||
if (outStream && JSON.parse(outStream).tags) { | ||
return JSON.parse(outStream).tags; | ||
const response = await axios.get('https://mcr.microsoft.com/v2/azure-cli/tags/list'); |
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.
Since this action runs on Node 20, native fetch is available and wouldn't require an additional dependency.
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.
I am happy to try it out again from a branch on my workflow if that helps and you want to test the change.
EDIT: Running...
Sorry, our run will be delayed, because we just managed to deploy a bug and the workflow will run a lot longer than normal
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.
I ran against this branch once use fetch instead was merged and tests were green:
I don't see the warning anymore and the action appears to be working as expected on our self-hosted runners ✅
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.
I don't know if I am allowed to approve, but here I am... :)
Hi @jakub-lesniak-ikea, does it work for you with |
No it does not, that was the reason I started testing the |
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.
Tested & it solves the original issue. I just had a very similar one related to my code which confused me cause I saw the same error messages before & after.
@MoChilia can you tell me/us what the process and requirements are for approval to merge this? Do we still have to do something? |
@luc122c, @MeikeMertschFortum, @jakub-lesniak-ikea, it has now been merged into |
Thank you for your help |
Fix the issue #153, replace
curl
with aGET
request to prevent stdout cutoff in Azure CLI versions, avoiding warnings in workflows.Close #153