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

PySpelling output artifact? #68

Closed
riccardoporreca opened this issue Oct 25, 2021 · 9 comments · Fixed by #96
Closed

PySpelling output artifact? #68

riccardoporreca opened this issue Oct 25, 2021 · 9 comments · Fixed by #96
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@riccardoporreca
Copy link
Contributor

First of all, thanks for the great work with this action! 👏

This might be a tricky one, but would it be possible to offer the possibility of dumping the output from pyspelling to a file (in addition to what we see in the workflow log), to allow uploading it as artifact in workflows using this action?

@jonasbn jonasbn self-assigned this Oct 31, 2021
@jonasbn jonasbn added the question Further information is requested label Oct 31, 2021
@jonasbn
Copy link
Collaborator

jonasbn commented Oct 31, 2021

Hi @riccardoporreca

Let me give it some thought.

@jonasbn jonasbn pinned this issue May 15, 2022
@jonasbn
Copy link
Collaborator

jonasbn commented May 15, 2022

Hi @riccardoporreca

I have done some experimenting and I came to a solution using tee.

Currently it captures all the output of the run of pyspelling not the action as a whole.

I am working out the details around the preservation of the generated artifact, until now to no avail.

You can see the master branch builds issue the warning:

No files were found with the provided path: /tmp/run-report.txt. No artifacts will be uploaded.

And no artefacts are available yet.

zsh> gh api \
  -H "Accept: application/vnd.github.v3+json" \
  /repos/rojopolis/spellcheck-github-actions/actions/artifacts
{
  "total_count": 0,
  "artifacts": []
}

I could get the artifact generated locally using Docker and I could even inspect it, but testing the action integration, requires some more work and working directly on the repository - not my favorite cup of tea, but I hope I can get it to work

@riccardoporreca
Copy link
Contributor Author

@jonasbn, thanks for looking into this!

I can have a look myself, I have quite some experience with Docker and it is not surprising the file is not found. I do not have much experience with Docker container actions but I can see how I can help out.

@jonasbn
Copy link
Collaborator

jonasbn commented May 16, 2022

Hi @riccardoporreca

Any assistance would be much appreciated. I am currently looking into how I can extract the generated run-report.txt from the Docker image so it can be made accessible as an artifact. I have been sleeping and showering on the problem and I will see if I can tweak the Docker Extract action to do what we need.

My current progress is available in the master branch and it is not complete. I will probably create a separate prototype for my investigation and roll back the changes in the master branch, since it was more complicated that expected and I do not want to disrupt the projects possible progress.

@riccardoporreca
Copy link
Contributor Author

@jonasbn, I can come up with something more concrete in the coming days, but overall my 0.02 $ here are that we need to produce the artifact inside the container in a mounted volume from the runner, so it is accessible after docker has run.

For completeness, the list of mounted volumes (-v <path-on-runner>:<path-inside-container>) is logged in the workflow run
https://github.com/rojopolis/spellcheck-github-actions/runs/6442126692?check_suite_focus=true#step:4:5

-v "/var/run/docker.sock":"/var/run/docker.sock" 
-v "/home/runner/work/_temp/_github_home":"/github/home"
-v "/home/runner/work/_temp/_github_workflow":"/github/workflow" 
-v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" 
-v "/home/runner/work/spellcheck-github-actions/spellcheck-github-actions":"/github/workspace"

Also note that the working directory inside the container is --workdir /github/workspace, which is a mounted volume for the working directory in the runner containing the checkout.

I guess you could naively specify a file in the working directory as tee spellcheck-output.txt (better be specific with the name), and similarly using path: spellcheck-output.txt in the upload artifact step.
Note that the working directory is in fact the repo's checkout directory, so this would produce an untracked file, which might be undesirable in some cases. I think it would be good to expose as action parameter the path to the output file, like output_file similar to the existing source_files. The parameter would be optional and no output would be produced if unspecified (to make this functionality opt-in w/o the risk of messing up with an untracked file the checkout directory).

@jonasbn
Copy link
Collaborator

jonasbn commented May 16, 2022

Hi @riccardoporreca

I will evaluate your proposal tonight. I think you cracked it 8-)

@jonasbn
Copy link
Collaborator

jonasbn commented May 16, 2022

Hi @riccardoporreca

I have made a successful implementation based on your guidance - thank you very much.

I will now update the documentation, so this new feature can be adopted by others.

Thank you for your contribution and patience.

Any additional feedback are of course most welcome.

@jonasbn jonasbn added this to the 0.24.0 milestone May 16, 2022
@riccardoporreca
Copy link
Contributor Author

Happy to help out @jonasbn!

A couple of additional notes from my side

(1) The way I hinted at exposing output_file is also how you are using it in your workflow (i.e. providing the actual name of the file):

output_file: spellcheck-output.txt

however the way it is implemented in the entrypoint (and how the input parameter is documented) always writes to spellcheck-output.txt
if [ -z "$OUTPUT_FILE" ]; then
pyspelling --config $SPELLCHECK_CONFIG_FILE $TASK_NAME $SOURCES_LIST | tee spellcheck-output.txt
else
pyspelling --config $SPELLCHECK_CONFIG_FILE $TASK_NAME $SOURCES_LIST
fi

(2) As documented in https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions

When you specify an input in a workflow file or use a default input value, GitHub creates an environment variable for the input with the name INPUT_<VARIABLE_NAME>. The environment variable created converts input names to uppercase letters and replaces spaces with _ characters.

The entrypoint appears to be working a.t.m. since if [ -z ] checks for empty strings, whereas we want to use -n to check for non-emptiness (see e.g. https://tldp.org/LDP/Bash-Beginners-Guide/html/sect_07_01.html)

Putting this together, besides updating the documentation to reflect the actual meaning of output_file, the entrypoint should rather be stg like this (including a nice message, that would also help with testing)

if [ -n "$INPUT_OUTPUT_FILE" ]; then
    echo "Producing spellcheck output fie >$INPUT_OUTPUT_FILE<"
    pyspelling --config $SPELLCHECK_CONFIG_FILE $TASK_NAME $SOURCES_LIST | tee $INPUT_OUTPUT_FILE
else
    pyspelling --config $SPELLCHECK_CONFIG_FILE $TASK_NAME $SOURCES_LIST
fi

@jonasbn jonasbn added enhancement New feature or request and removed question Further information is requested labels May 16, 2022
@jonasbn
Copy link
Collaborator

jonasbn commented May 16, 2022

Hi @riccardoporreca

I just saw your comment now, I wish I had seen it earlier. I believe I have addressed the issues you pointed out. I ran into them testing the implementation.

I will not release anything tonight since I am too tired, but I plan to release tomorrow.

You are welcome to give make a review of #96

@jonasbn jonasbn unpinned this issue May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants