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.

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

[BUG] Unable to output #460

Merged
merged 9 commits into from
Jul 29, 2023
Merged

Conversation

mickahell
Copy link
Collaborator

@mickahell mickahell commented Jul 27, 2023

Summary

Do not join logs by "\n" but instead used " " to keep them single line friendly with new GITHUB_OUTPUT variables.

Details and comments


Closes #458

@mickahell mickahell self-assigned this Jul 27, 2023
@mickahell mickahell added the bug Something isn't working label Jul 27, 2023
@mickahell
Copy link
Collaborator Author

mickahell commented Jul 27, 2023

@frankharkins I found why and fixed it :) ready for your review

Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

Thanks! What do you think about adding a guard to set_actions_output to assert there's no newline characters? Might make it easier to debug in the future.

@mickahell
Copy link
Collaborator Author

mickahell commented Jul 28, 2023

@frankharkins do you mean something like :

if "\n" in my_string:
   single_ value = " ".join(line.strip() for line in my_string.splitlines())

?

The thing is we should not break the submission parser, especially the description. I'm telling you that because is was one of my first solution but I broke the submission parser output ^^' (commit 57a9c4654e053fe5111a5e565c017e41f5770a03)

@frankharkins
Copy link
Member

frankharkins commented Jul 28, 2023

I was thinking more

assert "\n" not in my_string, f"Error: Newlines in github output ({my_string})"

so we can see what caused the problem in the action log

@mickahell
Copy link
Collaborator Author

Oh yeah a test case, well see :)
I'll try that

@mickahell
Copy link
Collaborator Author

@frankharkins I applied your suggestion :)
and I did some tests as well : actions 5699620350 and 5699614358 everything is working as expected

Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you :)

@frankharkins frankharkins merged commit ad90b34 into Qiskit:main Jul 29, 2023
4 checks passed
@mickahell mickahell deleted the bug/unable-output branch August 3, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Unable to process Output
2 participants