Use github access token when running docker locally #10056
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srietkerk
reviewed
Jul 30, 2024
cli/buildengine.ts
Outdated
| @@ -387,14 +387,29 @@ function runDockerAsync(args: string[]) { | |||
| if (process.platform == "darwin") | |||
| mountArg += ":delegated" | |||
|
|
|||
| let fullArgs = ["--rm", "-v", mountArg, "-w", "/src"].concat(dargs).concat([cs.dockerImage]).concat(args); | |||
There was a problem hiding this comment.
All of these concats look a little funky to me. Can we use spread syntax here instead?
| updateEngineAsync: () => runDockerAsync(["yotta", "update"]), | ||
| buildAsync: () => runDockerAsync(["yotta", "build"]), | ||
| updateEngineAsync: () => runDockerYottaAsync(["yotta", "update"]), | ||
| buildAsync: () => runDockerYottaAsync(["yotta", "build"]), |
There was a problem hiding this comment.
If we are special casing Yotta, do we need to include "yotta" in the args?
There was a problem hiding this comment.
I think it's easier to understand with yotta there since that's the command you would be running if you were doing all this manually.
srietkerk
approved these changes
Jul 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One annoying thing about using our CLI to build locally w/ docker images is that yotta is constantly getting throttled by GitHub. We even have a GITHUB_ACCESS_TOKEN environment variable that we advertise as something you can configure, but it only gets used by PXT and not yotta which is a bummer.
Well, not anymore! This PR passes that GITHUB_ACCESS_TOKEN variable to the docker container and runs a script to setup the local
~/.yotta/config.jsonand~/.netrcfiles to prevent throttling. This token doesn't actually need to have any permissions at all, GitHub just wants to use it for authentication.Also, this PR fixes the
--localflag which I had accidentally disabled in my last pr.Completely unrelated to this, I also converted a function in
cli.tsto use async await because it was bothering me.