langtech-bsc / magma

Apache License 2.0
0 stars 0 forks source link

fix bug on saving env variables to job.env file #10

Closed ankush13r closed 2 weeks ago

ankush13r commented 3 weeks ago

Issue: #9 modified env | grep -e '^JOB_' -e '^GPFS_' > src/job.env to env | grep -e '^JOB_' -e '^GPFS_' | awk -F= 'length($2) > 0' > src/job.env

Now if some variables are empty will be removed

PaulNdrei commented 3 weeks ago

Running https://github.com/langtech-bsc/llama3_jupyter launch job with Carme from speech team we noticed this error in error.log.

/gpfs/scratch/bscXX/bsc088XXX/jobs/llama3_jupyter/2/job.env: line 4: -N: command not found

I don´t know if there is any relationship with this issue. I think the tunnel command should be between quotes, maybe?

ankush13r commented 3 weeks ago

Yes, i also got the same error, this is if you don't add all variables required, but i think it will fix also your issue this part is not dynamic for saving variables with var.

      REMOTE_USER: ${{ secrets.REMOTE_USER }}
      REMOTE_GROUP: ${{ secrets.REMOTE_GROUP }}
      GPFS_SINGULARITY_IMAGE_REGISTRY_PATH: ${{ vars.GPFS_SINGULARITY_IMAGE_REGISTRY_PATH }}
      GPFS_JUPYTER_WORKING_DIR: ${{ vars.GPFS_JUPYTER_WORKING_DIR }}
      GPFS_MODELS_REGISTRY_PATH: ${{ vars.GPFS_MODELS_REGISTRY_PATH }}
      MLFLOW_TRACKING_SERVER_URL: ${{vars.MLFLOW_TRACKING_SERV
ER_URL}}

we have to try if we can do something like: cat vars >> $GITHUB_ENV

ankush13r commented 3 weeks ago

updated workflow using a action that moves all vars to env

- name: Set repository variables to env
        uses: oNaiPs/secrets-to-env-action@v1
        with:
          secrets: ${{ toJSON(vars) }}

Now it's fully ready to merge this pull request

PaulNdrei commented 3 weeks ago
  1. That action solves the issue of throwing an error when required secrets are empty? Referring to https://github.com/langtech-bsc/magma/issues/11

  2. Also, it's important to know if the GitHub Actions could reuse versioning; for security reasons, it would be better to ensure that oNaiPs/secrets-to-env-action@v1 cannot be modified and replaceable with a new v1 version with other code. I think @v1 is pointing to NaiPs/secrets-to-env-action v1.5.

ankush13r commented 3 weeks ago

It solves the current issue, but not throws the error. But it's a great idea to throw error, I'll try figure it out.

ankush13r commented 2 weeks ago

@PaulNdrei, Now it's ready to merge. Description: I've create a new node action that saves all variables or secrets to env and also if you specify the list of key must include, it will check them and throw error if any of them not exists.