rojopolis / spellcheck-github-actions

Spell check action
MIT License
132 stars 38 forks source link

PySpelling output artifact? #68

Closed riccardoporreca closed 2 years ago

riccardoporreca commented 2 years ago

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

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 commented 2 years ago

Hi @riccardoporreca

Let me give it some thought.

jonasbn commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

Hi @riccardoporreca

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

jonasbn commented 2 years ago

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.

riccardoporreca commented 2 years ago

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): https://github.com/rojopolis/spellcheck-github-actions/blob/3b6895555735993d4aaec3275862642b618ed9fe/.github/workflows/spelling_action.yml#L23 however the way it is implemented in the entrypoint (and how the input parameter is documented) always writes to spellcheck-output.txt https://github.com/rojopolis/spellcheck-github-actions/blob/3b6895555735993d4aaec3275862642b618ed9fe/entrypoint.sh#L91-L95

(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_. 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 commented 2 years ago

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