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

[feature] Introduce user defined variables or rename fileGroups to vars #1569

Open
artsiommiksiuk opened this issue Jul 19, 2024 · 10 comments
Open
Labels
enhancement New feature or request

Comments

@artsiommiksiuk
Copy link

artsiommiksiuk commented Jul 19, 2024

Is your feature request related to a problem? Please describe.

I have this task:

tasks:
  push-local:
    script: >
      aws ecr get-login-password --region eu-central-1 | docker login --username AWS --password-stdin xxxxxxxxxxx.dkr.ecr.eu-central-1.amazonaws.com &&
      docker tag my-image:latest xxxxxxxxxxx.dkr.ecr.eu-central-1.amazonaws.com/my-image:latest
      docker push xxxxxxxxxxx.dkr.ecr.eu-central-1.amazonaws.com/my-image:latest

There is at least two variables to extract:

ECR_REGISTRY: xxxxxxxxxxx.dkr.ecr.eu-central-1.amazonaws.com
ECR_REPOSITORY: my-image

Those are not files, but just an ordinary variables I need to use and substitute for the command to run, therefore using filesGroup feels kind awkward for this purpose.

Describe the solution you'd like

The title of the issue states this already. While filesGroup is called files, it actually behaves more like variables list section, because with the use of @group(var) token I can achieve variables substitution behaviour. Not ideal, but semantically means that fielsGroup is overspecified and can be reused for much broader automation.

So, instead:

filesGroup:
  files: "**/*.ts"

This should can be generalized like:

vars:
  filesGlob: "**/*.ts"

Because usage of this string is context dependent, and in one context it will be file in another it will be string. So we might have now situations like:

# similar to how @group behaves rn
@var(files) 
# now it's used as a glob
@glob(files)

Generally I'd say all the pieces are there - but semantics is overly specific.

Describe alternatives you've considered

Quite a big one.

Introduce own very minimal config language, as yaml pretty quickly becoming a limiting factor for me, as I have lot's of custom tasks to do in a polylanguage monorepo of js, ts, python and c++ (not a final list).

I don't want to open discussion about Starlark (I think it's kinda niche cult like garbage language), but approach which Just took seems very viable and flexible, and if it would've been possible to have much tighter integration with Just (either just in moon directly, or moon parses extended Just syntax files) it will bring automation with moon onto the whole new level.

Because besides variables, I'm actually missing an ability to solve lot's of different scenarios without offloading them into Just or bash or whatever external programming language I have, like detecting OS, small files generations fully embedded into build definition or based of complex templates, self-service tools, and anything which is missing in moon itself - I need to externalize it. Writing standalone plugin for that is kinda overkill.

@artsiommiksiuk artsiommiksiuk added the enhancement New feature or request label Jul 19, 2024
@artsiommiksiuk
Copy link
Author

Okay, I figured I can't use @group(files) as variable as it prefixes path. Then introducing @var(name) token might help mitigate this issue, but rn I don't know how to parameterize the commands except externalizing this into script.

@milesj
Copy link
Collaborator

milesj commented Jul 19, 2024

Variables are on the roadmap, but no ETA on when they will land.

@doron-cohen
Copy link

IMO it would be hard to generalise filesGroups into vars since they are treated as file globs. The benefit is that they could be checked for correctness (glob syntax for example).

I did suggest project specific metadata in #1571 which I think could solve the issue here but as a personal recommendation abusing it could cause the project's moon.yml to contain a lot of trash.

Since these are task specific variables why not use env like this. In the global tasks.yaml:

tasks:
  push-local:
    script: >
      aws ecr get-login-password --region eu-central-1 | docker login --username AWS --password-stdin $ECR_REGISTRY &&
      docker tag my-image:latest $ECR_REGISTRY/$ECR_REPOSITORY:latest
      docker push $ECR_REGISTRY/$ECR_REPOSITORY:latest

In the project's moon.yaml:

tasks:
  push-local:
    env:
      ECR_REGISTRY: xxxxxxxxxxx.dkr.ecr.eu-central-1.amazonaws.com
      ECR_REPOSITORY: my-image

This should work and you only define these for the task needing these vars.

@milesj I don't know what are the planned vars but project global env could help defining these for multiple tasks.

@milesj
Copy link
Collaborator

milesj commented Jul 22, 2024

Yeah env var substitution is also possible. I always forget about that. It should inherit from the global env and task-level env.

@artsiommiksiuk
Copy link
Author

@doron-cohen, @milesj thanks for the hint! I as well forgot about envs. Don't like the idea to use envs for that (potential conflict in envs space), but it's better than nothing.

@artsiommiksiuk
Copy link
Author

@milesj can we have the workspace defined envs? Seems like current yaml schema says there are no envs field available on a workspace level yet (as pointed by @doron-cohen)

@doron-cohen
Copy link

How about task variables?

tasks:
  push-local:
    variables:
      ecr_registry:
        type: uri
        required: false
        default: xxxxxxxxxxx.dkr.ecr.eu-central-1.amazonaws.com
      ecr_repository:
        type: string
        required: true
    script: >
      aws ecr get-login-password --region eu-central-1 | docker login --username AWS --password-stdin $ecr_registry &&
      docker tag my-image:latest $ecr_registry/$ecr_repository:latest
      docker push $ecr_registry/$ecr_repository:latest

These could be very handy when defining global tasks. Another pro is that you could fail to run the task when a required var is not defined instead of running the task with an empty string value to the env var.

@artsiommiksiuk
Copy link
Author

@doron-cohen I think this was the response from @milesj

Variables are on the roadmap, but no ETA on when they will land.

@milesj
Copy link
Collaborator

milesj commented Jul 23, 2024

I plan to migrate to pkl from yaml, which supports variables natively: https://pkl-lang.org/main/current/language-reference/index.html#let-expressions

That's the main reason vars are on the backlog.

can we have the workspace defined envs? Seems like current yaml schema says there are no envs field available on a workspace level yet

I'll think about it.

@artsiommiksiuk
Copy link
Author

I plan to migrate to pkl from yaml

That's cool! Can't wait to see it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

3 participants