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

Why an extra setup action? #9

Open
LinqLover opened this issue Oct 8, 2020 · 3 comments
Open

Why an extra setup action? #9

LinqLover opened this issue Oct 8, 2020 · 3 comments

Comments

@LinqLover
Copy link
Contributor

Sorry for spamming. I wonder whether it is necessary to have an extra setup action, which does not seem to be the default solution. Asking for curiosity only: What's the reason you decided to build an extra setup action instead of a smalltalkCI action itself? Wouldn't the latter make it easier for GitHub to cache the action? :-)

@LinqLover
Copy link
Contributor Author

Hm, I just found another use case for using an extra setup action, if the single steps could be redistributed between the setup and the test task.

For my project, I use the turnstyle action to ensure some tests that access a global web service are not executed at the same time. So I run this action right before my turnstyle action:

name: smalltalkCI

on: [push, pull_request]

jobs:
  build:
    steps:
      - uses: actions/checkout@v2
      - uses: hpi-swa/setup-smalltalkCI@v1
        with:
          smalltalk-version: Squeak64-Trunk
      - name: Turnstyle (critical section)
        uses: softprops/turnstyle@v1
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      - name: Run test suite
        run: smalltalkci -s ${{ matrix.smalltalk }}

However, this leads to unnecessarily long delay times after the first smalltalkci job instance has run (e.g. for the push) because the second job instance does not start to prepare the Squeak image before the first job instance has quitted. I could speed up my Actions workflow if this critical section could be reduced by breaking the smalltalk ci job down into separate jobs for setup + testing.

What do you think? Would this be far too complex? :-) Also, if we can use better caching as described in hpi-swa/smalltalkCI#471, the steps described here would become redundant.

@fniephaus
Copy link
Member

Sorry for spamming.

All good, you're asking good questions :)

I wonder whether it is necessary to have an extra setup action, which does not seem to be the default solution.

What do you think the action should do? What smalltalkCI setup would be closer to a default solution?

Wouldn't the latter make it easier for GitHub to cache the action?

Sorry, but I don't understand why you need caching. If you think you need it for keeping your trunk images up-to-date, I'd say this is not something smalltalkCI users should care about at this point. A smalltalkCI build only needs a VM and an image, and images are relatively large in size (compare to source code for example). Imagine we have 1000 projects, each caching a 50MB trunk image... that's a 50GB cache size in total. If smalltalkCI provides a recent trunk image, we don't need all of this.

For my project, I use the turnstyle action to ensure some tests that access a global web service are not executed at the same time.

I don't completely understand your use case for turnstyle. But if you really need it, it seems necessary that you sacrifices build time for synchronization.

I could speed up my Actions workflow if this critical section could be reduced by breaking the smalltalk ci job down into separate jobs for setup + testing.

This sounds like a feature request for smalltalkCI, probably something like a --setup-only cmd line option. Could you please open a separate issue against smalltalkCI?

I hope I answered all your questions. If not, please let me know and feel free to file separate issues in the future. :)

Best,
Fabio

@LinqLover
Copy link
Contributor Author

What do you think the action should do? What smalltalkCI setup would be closer to a default solution?

There are two ideals to how smalltalkCI could be invoked from an Actions workflow:
The one extreme might be to trigger it out of the box, in one single job, without wasting more lines than necessary in the workflow configuration file. This would make it as simple as possible to add smalltalkCI to a new workflow.
The other extreme, which I came up with in #9 (comment), would be to maximize modularity of jobs ...:

I don't completely understand your use case for turnstyle. But if you really need it, it seems necessary that you sacrifices build time for synchronization.

This is true, but unfortunately, GH appears not to support a built-in way to limit concurrent jobs. See this thread for more details:
How to limit concurrent workflow runs - GitHub Actions - GitHub Support Community
So I could at least burn a bit less build time if we had this --setup-only option:

This sounds like a feature request for smalltalkCI, probably something like a --setup-only cmd line option. Could you please open a separate issue against smalltalkCI?

See hpi-swa/smalltalkCI#479. :-)

Wouldn't the latter make it easier for GitHub to cache the action?

Sorry, but I don't understand why you need caching.

I admit I do not yet know what is happening behind the scenes on GitHub Actions to speed up builds, but I could imagine that they would provide docker images or any other kind of cacheable environment to reuse the dependencies for frequently used actions. However, this action (setup-smalltalkCI) appears to manually trigger apt-get which is another time-consuming operation ... If I compare this action to other actions I've met so far (e.g. 1466587594/get-current-time or actions/create-release), they do not require an extra installation process. Either they don't have any dependencies at all, or there must happen some magic from the side of GitHub Actions. But I'm noticing my own impreciseness at this point, I just do not yet know about GH Actions. :-)

However, the other point is the caching of images, which probably should be discussed here instead: hpi-swa/smalltalkCI#471

If smalltalkCI provides a recent trunk image, we don't need all of this.

+1!

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

No branches or pull requests

2 participants