Thread
#github-actions-kotlin-dsl
    v

    Vampire

    2 months ago
    Another idea regarding the typing spec. I'm currently adding another valid
    distribution
    value to my action. It would be nice if you somehow could define the valid enum values by supplying some regex and split pattern or similar. This way the valid enum values could be derived from the description of the in-/output. Currently it could happen that I only add it to the description or only add it to the typing file and the lists are different. For examle in my
    action.yml
    I have
    inputs:
        distribution:
            description: |
                The WSL distribution to install, update, or configure.
                Valid values: 'Alpine', 'Debian', 'kali-linux', 'openSUSE-Leap-15.2', 'Ubuntu-22.04', 'Ubuntu-20.04', 'Ubuntu-18.04', 'Ubuntu-16.04'
            required: false
            default: Debian
    So I could in the
    action-types.yml
    maybe write instead of
    inputs:
        distribution:
            type: enum
            allowed-values:
                - Alpine
                - Debian
                - kali-linux
                - openSUSE-Leap-15.2
                - Ubuntu-22.04
                - Ubuntu-20.04
                - Ubuntu-18.04
                - Ubuntu-16.04
    something like
    inputs:
        distribution:
            type: enum
            allowed-values:
                match-regex: "(?<=^Valid values: ').*(?='$)"
                split-split: "', '"
    or similar. The validation action could verify that there are some values, and the future code generator should always get the full list of options.
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    ah, this is the consequence of having the typings in a separate file 🙂 I fully get the intent, but let's discuss if it's the best approach
    ideally, the typings should be the source of truth about allowed values. Before considering adding some extra complexity for the typings: I can imagine input's description redirecting the user to the typings definition where they could check allowed values ("for allowed values, see action-typing.yml")
    ⬆️ this is for "classic" users, and for the type-safe users the enum would be the documentation
    WDYT?
    another approach that I use in a similar case (two files having the same value, but since they use another "technology", it's not possible to include one in another): have a custom validation logic that would validate if both places have consistent value. See https://github.com/krzema12/github-actions-kotlin-dsl/blob/6ac062f7761acff47fda0b5383c72a1a3df8a6cb/library/build.gradle.kts#L161-L172 - we validate that the same lib version is given in 3 places in total
    v

    Vampire

    2 months ago
    ah, this is the consequence of having the typings in a separate file
    I don't think so. It would be duplicated information either way. Other tools (like the current code generator of your action, maybe also others) or GitHub themselves would not respect the list even if it is in the same file, so it would be necessary in the description anyway to reach the end user.
    ideally, the typings should be the source of truth about allowed values.
    I agree generally but want to avoid having to generate the
    action.yml
    from the
    action-typing.yml
    . And redirecting the users is imho not a good option. This works for users reading the actual
    action.yml
    and it works for users using your DSL. But will not work for other cases like other DSL providers that generate code and docs from the
    action.yml
    and so on. Unless they add explicit support for the
    action-typing.yml
    it is more likely that they display the standard description like you also do currently. I personally prefer to have the info like if your custom addition would not exist so that all others can just treat it like usual. But that's just my PoV of course. 🙂 Of course it could also be solved with some custom validation that verifies the lists are the same. That could then also check that it is the same in the readme. (while thinking about it, I should generate the list into the readme as I have a preprocessing step for it anyway) I just thought it might be convenient to take the values from the official source of truth, which is the description of the in-/output.
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    got it, thanks for this idea - for now I'd hold with implementing it because it will lead to action-typing.yml not being self-contained, which has its consequences
    are you OK with going forward with how the typings work now? I'll add a task for this idea so that we don't lose it, and will revisit in e.g. several months after hopefully there's higher adoption of the typings lib
    v

    Vampire

    2 months ago
    Yeah, I'm fine with it so far, was just an idea I got while adding that new value. 🙂 Regarding the PR to setup-wsl, should I merge it now (after adding the additional new value) or should we keep it as PR for now?
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    I'll add this extra value now in the same PR
    v

    Vampire

    2 months ago
    You can, I would have done it too, doesn't matter much. The question was whether we want it actually merged. 😃
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    no worries - done 🙂 yes, it would be cool to have it merged, this way your action becomes the first one to have these typings integrated and we can work on the code generator part (@jmfayard yay!)
    could you run (approve) the workflows just one more time before merging?
    v

    Vampire

    2 months ago
    Yeah, sure, a bit annoying that this is necessary on each push
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    I think it's because it's my first PR in this repo, in the next ones I'll be "trusted by GitHub ™️ " 🙂
    BTW I noticed that some of the checks fail in a non-deterministic way, see e.g. https://github.com/Vampire/setup-wsl/runs/7129840781?check_suite_focus=true
    v

    Vampire

    2 months ago
    I think it's because it's my first PR in this repo, in the next ones I'll be
    Yep, just thought it would be enough to enable builds by you one time and not on every push. 🙂 That's caused by the tests actually being "system tests" that depend on external services. If those are unreliable, the test might fail. Actually if such outages accumulate, I add some counter-measure to the action. For example getting the download URL where it is not static, or refreshing the openSUSE packages. Both were pretty flaky, so I added retry mechanisms for those. The tests run every night to capture problems introduced by the environment. The case you linked to is somewhat different. I abuse a bit the
    actions/cache
    action to build the action one time in one job and restore the result in the other jobs for local execution of the action. This way I don't need to rebuild the action for each test job, especially as I do not want the build result in the normal source tree either. I don't even like having it in VCS at all, but well, that's how GHA is designed to be distributed, but I at least just have the build result in a separate branch. I could alternatively publish the build result as artifact and then download it in the other test jobs, but then I have a build artifact which is not really proper. Now the problem is, that
    actions/cache
    sometimes fails to retrieve the cache, for example due to connection problems. The action assumes a cache is not important and can be rebuilt, so it just continues and then fails as the action is not found. There is not too much I can do about that, except for using an own cache action that would fail on restore already.
    It's merged, thx. Btw. do you add the additional value also to the wrapper or do you want a PR?
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    I'll add it tomorrow probably - if you have a spare moment earlier, I'd appreciate a PR 😃
    v

    Vampire

    2 months ago
    Maybe not. Two days to vacation start, work to do, and I want to get the release out with the new distribution. 🙂
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    I thought about this idea of extracting allowed enum values from description. I actually like it, but I'd like to propose another implementation. I'll prepare a PoC/example, then share it here
    v

    Vampire

    2 months ago
    Piotr Krzemiński

    Piotr Krzemiński

    2 months ago
    Awesome, thanks for both! 😃