iterative / cml

♾️ CML - Continuous Machine Learning | CI/CD for ML
http://cml.dev
Apache License 2.0
4.04k stars 339 forks source link

public access to a dvc-cml project? #43

Closed elleobrien closed 4 years ago

elleobrien commented 4 years ago

@dmpetrov and I have been talking about how we'll build tutorials for dvc-cml. One idea, which I've been building in a repo, is a project where anyone can make a fork and then submit a PR to see the workflow in action.

However, I've found this note on the Settings/Secrets page:

Secrets are not passed to workflows that are triggered by a pull request from a fork. Learn more.

If I understand correctly, this means that if someone in the public/outside DVC cloned our repo and attempted to make a PR, dvc repro might be triggered BUT the runner would not be able to access credentials, such as the Google Drive credentials needed to push/pull project artifacts. Does this sound correct?

If it's an issue, seems like we could simply put the credentials in a config file in the repo- I think, with GDrive, this is often alright?

shcheklein commented 4 years ago

It's absolutely possible and safe to give public access to push branches I suppose in this case.

put the credentials in a config file in the repo- I think, with GDrive, this is often alright?

you mean JSON file? it means that people will have read/write access to that Drive. It's probably fine if we create some demo account. But it's not safe in general case.

dmpetrov commented 4 years ago

@andronovhopf what provides better user experience from the blog post point of view? The Demo account credentials or giving access for PRs in our repo? Or both? 😄

elleobrien commented 4 years ago

@dmpetrov I think giving access to the PRs is the main thing I'm interested in.

Just to clarify- if, as @shcheklein said, we did give public access to push branches, will we have to worry about secrets not being passed to workflows "triggered by a pull request from a fork"?

shcheklein commented 4 years ago

will we have to worry about secrets not being passed to workflows "triggered by a pull request from a fork"?

yes, we won't have to worry as far as I remember ... i would ask @efiop and @DavidGOrtega to double check.

efiop commented 4 years ago

I didn't spend enough time with github actions yet, but with things like trravis secrets are indeed not passed to PRs that are not coming from upstream branches. Not sure what secrets we currently use for crml action and whether or not they could be put into a public config, so can't say anything for sure here :slightly_frowning_face: CC @DavidGOrtega

DavidGOrtega commented 4 years ago

This is a Github (and possibly any other vendor) limitation. Why maybe don't just set the variables not as a secret? If anyone can upload there an specific google account with Gdrive for that project would work.

Another solution would be storing the variables in the project and setup them if the Github event is running against your project. That's the way the Pillow guys have resolved this.

   - name: Prepare coverage token
      if: success() && github.repository == 'python-pillow/Pillow'
      run: cp .github/codecov-upstream.yml .codecov.yml

    - name: Upload coverage
      if: success()
      uses: codecov/codecov-action@v1
      with:
        token: ${{ secrets.CODECOV_TOKEN }}
        name: ${{ matrix.os }} Python ${{ matrix.python-version }}

I can help you to setup something like the last thing if you need it

elleobrien commented 4 years ago

hmm so jorge made a PR for a test repo I made and it did trigger a Git Action, but it seems that the credentials from the secret are not getting passed:

Google drive DVC remote found but no credentials found
Pulling from DVC remote ...

so the job won't complete. We'll have to figure something out as @DavidGOrtega suggested if we want to use this workflow in a tutorial- assuming everyone is okay from a security POV to have the client ID/secret/access codes for the project GDrive folder exposed in this case? @shcheklein @efiop

DavidGOrtega commented 4 years ago

Has this been addressed @andronovhopf ? Is there anything we can do o our side?

elleobrien commented 4 years ago

Hi @DavidGOrtega it won't work; to do dvc push/pull there's no way around needing to set credentials as Git secrets that I can find. We'll just have users setup credentials for themselves. So nothing to be done from the CML side!