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

feat: env to use path params in download url #19526

Merged
merged 8 commits into from Jan 18, 2022
Merged

feat: env to use path params in download url #19526

merged 8 commits into from Jan 18, 2022

Conversation

HarlesPilter
Copy link
Contributor

User facing changelog

Add CYPRESS_DOWNLOAD_PATH_PARAMS env variable to use path based params instead of query based params in download url.

Additional details

  • Why was this change necessary? - After Add Paths and .zip to Download URLs #17291 was merged there was no way to switch the download url form query based parameters to path parameters.
  • What is affected by this change? -> Cypress CLI. Support for new env variable

How has the user experience changed?

If the CYPRESS_DOWNLOAD_PATH_PARAMS is set to true then cypress is downloaded using path based params(https://download.cypress.io/desktop/0.20.2/OS-ARCH/cypress.zip)
If the new env variable is not used then download url doesn't change (uses query based params https://download.cypress.io/desktop/0.20.2?platform=OS&arch=ARCH)

PR Tasks

@HarlesPilter HarlesPilter requested a review from a team as a code owner January 3, 2022 19:26
@HarlesPilter HarlesPilter requested review from jennifer-shehane and removed request for a team January 3, 2022 19:26
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 3, 2022

Thanks for taking the time to open a PR!

@mjhenkes
Copy link
Member

Hi @HarlesPilter, thank you for taking the time to contribute! This is a good start and definitely will help us cover a gap where the users download proxy isn't 1:1 with our download server. I would ask you to expand the scope of the change a bit though.

With this PR we're covering the case where the version of cypress the user wants to download is located at exactly ${endpoint}/${platform}-${arch()}/cypress.zip but I can't be sure that is covering all use cases as urls could vary wildly between different setups.

I'd like to propose that instead of taking a boolean we take a string template. That way a user can exactly specify how they want their download url constructed.

For example:
We change your ENV from CYPRESS_DOWNLOAD_PATH_PARAMS to CYPRESS_DOWNLOAD_PATH_TEMPLATE

And You'd set CYPRESS_DOWNLOAD_PATH_TEMPLATE='${endpoint}/${platform}-${arch()}/cypress.zip'

The logic would update to check if the CYPRESS_DOWNLOAD_PATH_TEMPLATE is valued, then sub in endpoint, platform and arch, then use the download url. This would allow for all kinds of different download paths

  • '${endpoint}/${platform}-${arch()}/cypress.zip' Your version
  • '${endpoint}/${platform}/${arch()}/cypress.zip' slash separating architecture
  • '${endpoint}/cypress.zip' completely ignoring platform and architecture for some reason.

@HarlesPilter
Copy link
Contributor Author

Good idea! Implemented the CYPRESS_DOWNLOAD_PATH_TEMPLATE, but I used ${arch} in the template instead of ${arch()}

@ryanthemanuel ryanthemanuel self-requested a review January 14, 2022 16:35
@mjhenkes mjhenkes merged commit 2685f82 into cypress-io:develop Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installing Cypress with CYPRESS_DOWNLOAD_MIRROR and Artifactory
3 participants