Fix ci outputs

[nineinchnick]

Description

Fix referring to outputs from the same job.

Stop using the deprecated set-output command: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Broken since #10984. Supersedes #14661

Non-technical explanation

n/a

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

[findinpath]

Can we have yes/no instead of true/false to avoid misunderstandings / future confusions with boolean type?

[nineinchnick]

In YAML, that's not a good idea, because no also gets typecasted into a boolean, whereas yes is not. So it's actually worse than using true and false where both get cast into a boolean if not quoted.

IMO in YAML, explicit quoting is the right thing to do and should be enough.

But here we already only treat this as a string.

[findinpath]

The outcome in this case a boolean right?

(ignore exclusion if is set to "false")

[nineinchnick]

No, it's a string. I think it might be caused by using the multiline block (using >-, where > is replacing newlines with spaces and - chops the trailing newline).

You can see the final yaml after evaluating the GHA expressions in the step that writes it into the file:

and then you can see how it gets read into Python in the next step:

[hashhar]

I've run CI with secrets here - #14664

[hashhar]

Can you clarify in 2nd commit message that "Using output of a step within the same job doesn't work as expected/is not supposed to work" (and probably split out removal of set-output in separate commit otherwise it's unclear to anyone in future what the issue was and it was unrelated to set-output being deprecated)

[nineinchnick]

Can you clarify in 2nd commit message that "Using output of a step within the same job doesn't work as expected/is not supposed to work" (and probably split out removal of set-output in separate commit otherwise it's unclear to anyone in future what the issue was and it was unrelated to set-output being deprecated)

All done!

[nineinchnick]

@hashhar I saw in your test PR with secrets that it still didn't work. I restored the outputs definition in the job, maybe it's required. Can you run it again?

[nineinchnick]

@hashhar It didn't work again. I simplified it by removing all vars and just checking secrets directly when rendering the matrix yaml. Please update your PR.

[hashhar]

Was a small syntax error - pushed a fix. Will merge after verifying things work as expected (and will try to send a follow-up to clean this complexity up if possible).

[nineinchnick]

Thanks, looks like it's finally working: https://github.com/trinodb/trino/actions/runs/3271424004/jobs/5381195377

will try to send a follow-up to clean this complexity up if possible.

Do you mean doing something with these ignore exclusion if? That'd be nice, they're hard to read.

[hashhar]

Do you mean doing something with these ignore exclusion if? That'd be nice, they're hard to read.

Indeed. First of all that name is very misleading + I don't like how we try to emulate some boolean logic instead of actually showing how GHA matrix exclusions work (which is simpler than this "fake" boolean stuff and can be explained in a code comment + link to docs).

[ebyhr]

@hashhar Can we merge this PR now?

[hashhar]

@ebyhr Yes, I think so - I don't know if failures are related. Sorry I did not take a look at this recently.

[ebyhr]

Thanks for your confirmation. Let me merge so that we can run #14687 in Databricks.