Enable remote proxy binary downloads #2254
Conversation
🚫 Docs Preview DeniedYou must have approval from an Apollo team member to request a docs preview. If you are a team member, please comment File Changes 4 new, 8 changed, 3 removed |
There was a problem hiding this comment.
IIUC, the existing APOLLO_ROVER_DOWNLOAD_HOST env var sets the binary download URL for npm installation, while the new APOLLO_ROVER_BINARY_REMOTE would set the binary download URL for Bash and PowerShell. We need distinct env vars because the download URLs are usually different, but they serve similar purposes.
Given the above, it'd be ideal if the name of the new env var:
- Conveys it's a download URL for the specific Bash/PowerShell use case of downloading from a GitHub proxy/mirror
- Is consistent and/or uses the same namespacing as the existing env var, because they're for similar purposes.
For 2, we could use the existing prefix of APOLLO_ROVER_DOWNLOAD_*. For 1, we could append a suffix that conveys it's a GitHub proxy or remote mirror, something like:
APOLLO_ROVER_DOWNLOAD_GITHUB_PROXYAPOLLO_ROVER_DOWNLOAD_PROXY_HOSTAPOLLO_ROVER_DOWNLOAD_GITHUB_HOSTAPOLLO_ROVER_DOWNLOAD_GITHUB_MIRROR
thoughts @Meschreiber ?
There was a problem hiding this comment.
Great point, thanks for taking a look so quickly @shorgi !
I'm fine with any of the above. To decide, one question for @LongLiveCHIEF :
Does the download URL for Bash and PowerShell always have to be a GH repo URL? (Will it work with other binary URL sources?)
Besides that, I like the inclusion of PROXY to make clearer how it's different from DOWNLOAD_HOST.
There was a problem hiding this comment.
I worry that the terminology "proxy" will be easily conflated with the more "classical" HTTP proxies (e.g., those corporate SOCKS type) which would require different configuration.
|
I sent @Meschreiber some extra details on Slack because I figured you may
want to rename things. Wherever I put `_BINARY_REMOTE` the asset comes from
GitHub. Wherever `_DOWNLOAD_HOST` is used, it's from orbiter endpoint.
Any of the above makes sense to me, but I also wonder... are the binaries
the same? If so, why not just use the same endpoint for all installers, and
get rid of the complexity of having env variables for multiple different
endpoints?
…On Fri, Nov 15, 2024, 1:17 PM Edward Huang ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
On docs/source/getting-started.md
<#2254 (comment)>:
IIUC, the existing APOLLO_ROVER_DOWNLOAD_HOST env var sets the binary
download URL for npm installation, while the new
APOLLO_ROVER_BINARY_REMOTE would set the binary download URL for Bash and
PowerShell. We need distinct env vars because the download URLs are usually
different, but they serve similar purposes.
Given the above, it'd be ideal if the name of the new env var:
1. Conveys it's a download URL for the specific Bash/PowerShell use
case of downloading from a GitHub proxy/mirror
2. Is consistent and/or uses the same namespacing as the existing env
var, because they're for similar purposes.
For 2, we could use the existing prefix of APOLLO_ROVER_DOWNLOAD_*. For
1, we could append a suffix that conveys it's a GitHub proxy or remote
mirror, something like:
- APOLLO_ROVER_DOWNLOAD_GITHUB_PROXY
- APOLLO_ROVER_DOWNLOAD_PROXY_HOST
- APOLLO_ROVER_DOWNLOAD_GITHUB_HOST
- APOLLO_ROVER_DOWNLOAD_GITHUB_MIRROR
thoughts @Meschreiber <https://github.com/Meschreiber> ?
—
Reply to this email directly, view it on GitHub
<#2254 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZBLAMWHRT3SR7QE5LHXT32AZCDZAVCNFSM6AAAAABRLN6QROVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMZZGQZTGMJSG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Just a point to note, the host is a proxy if you set it, the same way. That
remote is a proxy if you set it. I think if you make one say proxy, they
should both say proxy, to limit the confusion.
I don't think it's good to think of them as exclusively used for
bash/powershell variables, because they could very easily wind up being
used for plugins or other installers.
It doesn't matter what the eventual URL is, that's entirely up to you guys.
Both _REMOTE and _HOST are ways for users to declare a proxy to use... and
that proxy will be configured by the users networks to connect to whatever
URL the installer (npm, bash, cargo, etc...) uses by default.
…On Mon, Nov 18, 2024, 10:35 AM Maria Elisabeth Schreiber < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
On docs/source/getting-started.md
<#2254 (comment)>:
Great point, thanks for taking a look so quickly @shorgi
<https://github.com/shorgi> !
I'm fine with any of the above. To decide, one question for @LongLiveCHIEF
<https://github.com/LongLiveCHIEF> :
Does the download URL for Bash and PowerShell always have to be a GH repo
URL? (Will it work with other binary URL sources?)
Besides that, I like the inclusion of PROXY to make clearer how it's
different from DOWNLOAD_HOST.
—
Reply to this email directly, view it on GitHub
<#2254 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZBLANBUAAB7L2FYV3K2MD2BIJNZAVCNFSM6AAAAABRLN6QROVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINBTGE2TAOBVHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thanks @LongLiveCHIEF -- so is the keyword here |
Yep, that would totally work. If that works for you, then I will rebase and update my PR's to use
|
|
@LongLiveCHIEF , works for me, thank you! 🙇♀️ |
|
@Meschreiber @shorgi rebase complete on both PR's, using suggested syntax. |
This PR adds the ability to specify a proxy/mirror remote for the binary installation methods for apollo rover. This functionality was added for the npm installer in #1713 and #1675, and is being added for the router binary installation script in apollographql/router#6244