-
Notifications
You must be signed in to change notification settings - Fork 13
debug options for validation #51
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
Conversation
|
Looks like versions of CML do not support |
Thats strange. Which version of CML does not support --version? |
|
@DavidGOrtega it had failed on 0.3.0 @0x2b3bfa0 good catch. |
* pass with no sudo * fail when unable to parse version
|
Makes good sense to me, Is this the preferred course? @casperdcl
…On Thu, Oct 7, 2021, 09:25 Casper da Costa-Luis ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/utils.js
<#51 (comment)>:
> @@ -21,7 +21,10 @@ const setupCml = async opts => {
if (sudo) {
try {
sudoPath = await exec('which sudo');
- } catch (err) {}
+ } catch (err) {
+ console.log('Failed find the sudo binary.', err);
+ process.exit(1);
if we changed
https://github.com/iterative/setup-cml/blob/a7b00a80a68861804bbdd73396f56b81d5ce999f/action.yml#L11
to default: false & bumped the action version number, then it would be
fine to fail upon sudo not found.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#51 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIN7M67F6W74NRFDJ3JZALUFXCYNANCNFSM5ERX4R2Q>
.
|
|
@0x2b3bfa0 what do you think with these updates? |
|
@dacbd sorry for the super late reply, are you happy with it? |
|
@DavidGOrtega no worries, my fork has been sufficient for my own use/debugging. I have looked at the error in the checks and I'm not sure what's causing it? I'm fine with closing this and maintaining my own setup action to help with my testing, but if you feel it would be useful for others at all I would like to see it merged. |
|
I think is useful specially coming from you. What I do not get is why the original
Let me see 👀 |
❤️
CML 0.3.0 did not have a
Thanks, my inner voice has written it off as a weird GitHub cross-account ref thing. I did mess around with the action a bit (made a main branch) on mine but it has been long enough ago I don't remember what I was trying to do? 🤣 So these links might not be helpful, but it does spam my email every day... https://github.com/dacbd/setup-cml/actions v1...dacbd:main#diff-55f05888fd854d9962bdd4d8ccf23a07b610990f58174397b2586b58f8937428 |
|
need to resolve conflicts @dacbd :) |
|
I think we can close and revisit later, nice to have but not required. |


I added a simple sanity log to assist with any future debugging or inspection in GitHub action logs.
I believe that most users especially when constructing their initial tooling aren't pinning the version of CML to install.
My reasoning for this addition comes from the recent release of CML 0.7.0 -> 0.7.1, there was a discussion on discord about an error the was patched, I encountered a nearly identical error and wanted to be certain what the
latestversion was...example output:
