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

Read script stdout and wrap it in required json format #31

Merged

Conversation

andywaltlova
Copy link
Collaborator

@andywaltlova andywaltlova commented Jun 30, 2023

HMS-2005

Replace dumb read of json file with json containing correlation_id and stdout

Content of file that is sent to api/ingress/v1/upload (in local development proxied to minio) with output from date command looks like this:

{"correlation_id":"00000000-0000-0000-0000-000000000000","stdout":"Fri Jun 30 08:05:14 UTC 2023\n"}

Works same with /usr/bin/convert2rhel --help see attached file - 66ZLDMZKn4-000001.txt

@andywaltlova andywaltlova force-pushed the return-json-file-with-script-stdout branch from 4392f05 to b751d6c Compare June 30, 2023 08:38
@andywaltlova andywaltlova force-pushed the return-json-file-with-script-stdout branch from 5155c01 to 7c073f3 Compare June 30, 2023 11:00
@andywaltlova andywaltlova force-pushed the return-json-file-with-script-stdout branch from 7c073f3 to 52968ba Compare June 30, 2023 13:20
Comment on lines 4 to 7
echo "BEGIN MARKER"
echo "$data"
echo "END MARKER"
/usr/bin/convert2rhel --version
Copy link
Member

Choose a reason for hiding this comment

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

I would only invert the order of execution here... Ideally, the marker will be the last thing in the output, and the tool that is running will come just before it.

Thinking about the idea that the script will "read" the output of the tool that just run, I think it makes sense that the tool runs before the marker. Let me know what you think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I have no problem with that, when I was reading the description I just though that if it should be generic that It shouldn't matter to insights where the output is in the stdout as long as it is between the markers it will get shown. That is why I put command before and after the markers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Inverted, commands are firsts then the content within markers

Copy link
Member

@r0x0d r0x0d left a comment

Choose a reason for hiding this comment

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

Looks good to me. Only one comment about the order of execution of the initial script. other than that, I think we are ready to merge.

@andywaltlova andywaltlova force-pushed the return-json-file-with-script-stdout branch from 52968ba to 200c89f Compare July 2, 2023 12:47
@andywaltlova andywaltlova merged commit 9e1e7de into oamg:main Jul 3, 2023
@andywaltlova andywaltlova deleted the return-json-file-with-script-stdout branch August 17, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants