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

could a runner's cache be supported? #60

Open
2bndy5 opened this issue Apr 17, 2022 · 4 comments
Open

could a runner's cache be supported? #60

2bndy5 opened this issue Apr 17, 2022 · 4 comments
Assignees
Labels
type: enhancement Proposed improvement

Comments

@2bndy5
Copy link
Contributor

2bndy5 commented Apr 17, 2022

This question is what made me think of the #59 suggestion because it would aid in reducing the action's runtime. My intention is to make use of actions/cache@v3.

Initially, I thought to install the python src of this action as a step then execute the python code as another step, but it looks like that would be skipping the action's setup script.

So, this question seems dependent on where the cache-able assets exist (in the docker container's FS or the runner's FS). When I say "cache-able assets", I mean the Arduino cores (builtin or externally fetched like the ATTinyCore). I don't think that caching the size-delta reports would be applicable/helpful. Maybe the ArduinoCLI tool could also apply to the cache...

@per1234 per1234 added the type: enhancement Proposed improvement label Apr 18, 2022
@per1234
Copy link
Collaborator

per1234 commented Apr 18, 2022

Hi @2bndy5. I made some efforts to set up caching in all our standardized GitHub Actions workflows in preparation for widescale deployment to Arduino's repositories last year. However, I don't know a lot about the subject matter, was told that it was not worth the effort, and did not find information in my researches that would allow me to justify the significant increase in complexity and maintenance burden it added to the workflows. So I reverted it. I documented everything I know on the subject in that commit here: per1234/.github@f1cadde

I am certainly interested in the idea of making meaningful improvements to the efficiency of the action or workflows. I would give careful consideration to a well researched and defended proposal for any of the following:

Caching within the action

Ideally this would be done in a manner that separates GitHub Actions-specific operations from what is mostly not a GitHub Actions-specific Python script.

Documentation of caching in workflows using this action

This would be added to the action's documentation to serve as a reference to its users.

Caching in Arduino's standardized workflows

We have a collection of standardized workflows which are maintained in dedicated repositories:


in the docker container's FS

This action does not use a Docker container. It did during the earlier phase of development when that was the only option, but we decided to switch it to a "Composite Action" (originally called "composite run steps action") after GitHub made this option available. The action runs directly in the GitHub Actions runner machine.

@per1234 per1234 self-assigned this Apr 18, 2022
@2bndy5
Copy link
Contributor Author

2bndy5 commented Apr 18, 2022

@per1234 Its always a pleasure to talk "shop" with you! I wan't aware of the previous efforts. BTW, some of that commit's links (in the description) are now 404.

since GitHub is on AWS and arduino.cc is on AWS, which makes the download nearly instantaneous

This surprised me. I hadn't considered the source of the assets when opening this issue.

The action runs directly in the GitHub Actions runner machine.

This makes using optional cache completely possible (internally). After looking closely at the action_setup,sh script, I think there's nothing stopping me from executing this action's python src code as an individual step (given that most input options - including venv activation path - is a env var or an identified step output).

image

There's no need to burden this exploration on you alone. I can start some rudimentary experiments with RF24 lib and report my findings. Full disclosure, I'll probably just execute the action as a python script with env vars and pseudo step output vars to make use of actions/cache@v3.

It would really help measure the benefit of caching if/when #59 is resolved. I'm unsure of the contributing standards here, but I'm willing to contribute a simple solution for that.

@per1234
Copy link
Collaborator

per1234 commented Apr 18, 2022

some of that commit's links (in the description) are now 404.

Yeah, it is not a good commit message. I actually just did a copy/paste of a summary of the situation I provided someone I was asking for an opinion on whether the caching efforts should be reverted.

I'm willing to contribute a simple solution for that.

Contributions are welcome!

@2bndy5
Copy link
Contributor Author

2bndy5 commented Apr 21, 2022

Looking closer at what the setup script does (& what the python script does preliminarily), I think a cache is dependent on the path used for the venv and ArduinoCLI. Please note that I haven't run any tests yet, rather I'm just researching what this action needs to execute.

Thoughts about venv path

Installing python from deadsnake PPA may be the most time consuming. A breaking change idea is to use python installed from actions/setup-python (which is much faster because it uses github's octokit). But that might introduce regressions related to changes in python since v3.8 - there's also no way for the user to write a cache key based on a hash of this action's requirements.txt file. FWIW, PIO uses this approach to allow workflows to cache the installed python env's libraries, but they rely on pip to interact properly with a runner's cache.

Thoughts about Arduino CLI path

This is of lesser importance to me because it seems entirely dependent on whatever the latest Arduino CLI version is (unless specified by the user). If I wanted to cache this tool, then I'd probably rely on the arduino/setup-arduino-cli action as a workflow step that conditionally executes once a month (or maybe every 90 days).

Caching the CLI tool's necessary data (like cores, libs, etc) seems to be more important. However, it might be harder to write an appropriate cache key based on lib/core version(s) for this to happen properly - it might be better to just use a time-based cache key for this. The following seems like the pertinent paths for caching this data:

arduino_cli_user_directory_path = pathlib.Path.home().joinpath("Arduino")
arduino_cli_data_directory_path = pathlib.Path.home().joinpath(".arduino15")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Proposed improvement
Projects
None yet
Development

No branches or pull requests

2 participants