Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Issue 12304: Updating UPDATE_HOST to point to brave.com cname #12305

Merged
merged 1 commit into from
Dec 19, 2017

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Dec 16, 2017

fix #12304

Testing Notes: Geo logs for brave stats are captured using fastly. After this change download and update requests will go through brave domains which should go through fastly.net. Verify if stats is working correctly.

@codecov-io
Copy link

codecov-io commented Dec 16, 2017

Codecov Report

Merging #12305 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #12305   +/-   ##
=======================================
  Coverage   55.29%   55.29%           
=======================================
  Files         276      276           
  Lines       26740    26740           
  Branches     4310     4310           
=======================================
  Hits        14786    14786           
  Misses      11954    11954
Flag Coverage Δ
#unittest 55.29% <100%> (ø) ⬆️
Impacted Files Coverage Δ
js/constants/appConfig.js 100% <100%> (ø) ⬆️

@@ -5,8 +5,8 @@ const Immutable = require('immutable')
const {getTargetAboutUrl} = require('../lib/appUrlUtil')

// BRAVE_UPDATE_HOST should be set to the host name for the auto-updater server
const updateHost = process.env.BRAVE_UPDATE_HOST || 'https://brave-laptop-updates.global.ssl.fastly.net'
const winUpdateHost = process.env.BRAVE_WIN_UPDATE_HOST || 'https://brave-download.global.ssl.fastly.net'
const updateHost = process.env.BRAVE_UPDATE_HOST || 'https://laptop-updates.brave.com/'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no trailing slash

@diracdeltas diracdeltas assigned jumde and unassigned diracdeltas Dec 16, 2017
@jumde jumde force-pushed the changing-update-hosts branch from b1a4560 to 17632a4 Compare December 17, 2017 23:37
@jumde jumde assigned aekeus and jumde and unassigned jumde and aekeus Dec 17, 2017
@diracdeltas
Copy link
Member

this looks good to me but i will defer to @aekeus to confirm that these host cnames are stable for production

@diracdeltas diracdeltas added this to the 0.20.x (Beta Channel) milestone Dec 18, 2017
@aekeus
Copy link
Member

aekeus commented Dec 18, 2017

There is a current requirement to pass through Fastly for log file geolocation. This will break that process unless I am misunderstanding.

@jumde
Copy link
Contributor Author

jumde commented Dec 18, 2017

Filed #12320 to investigate alternate ways to collect logs.

aekeus
aekeus previously approved these changes Dec 19, 2017
Copy link
Member

@aekeus aekeus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good. We will confirm the Fastly geo location during testing.

@jumde jumde merged commit d621440 into master Dec 19, 2017
@bsclifton bsclifton deleted the changing-update-hosts branch December 19, 2017 21:22
@bsclifton
Copy link
Member

bsclifton commented Dec 19, 2017

@jumde for future reference, please make sure to share the SHA for each of the merges done 😄

For example, this PR was merged with:
master d621440
0.21.x d3dfcc9
0.20.x 114d2f4

Thanks! 😄 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating UPDATE_HOST to point to brave.com cname
5 participants