Skip to content

Conversation

@brizental
Copy link
Contributor

@brizental brizental commented Sep 13, 2021

It works: https://debug-ping-preview.firebaseapp.com/pings/node-js

Pull Request checklist

  • Quality: Make sure this PR builds and runs cleanly.
    • Inside the glean/ folder, run:
      • npm run test Runs all tests
      • npm run lint Runs all linters
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
  • Documentation: This PR includes documentation changes, an explanation of why it does not need that or a follow-up bug has been filed to do that work

@auto-assign auto-assign bot requested a review from chutten September 13, 2021 13:57
Copy link
Contributor

@chutten chutten left a comment

Choose a reason for hiding this comment

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

r+wc

// since it is validated upon Glean initialzation
// TODO: Bug 1730496
const parsedURL = new URL(url);
const mod = parsedURL.protocol === "http:" ? http : https;
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there's an interesting discussion to be had about allowing un-TLS'd connections in Glean to non-local hosts, but this isn't the time or place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There will be some validation soon https://bugzilla.mozilla.org/show_bug.cgi?id=1730496#c1

@brizental brizental force-pushed the 1728810-node-uploader branch from 0b681a9 to e5778dc Compare September 16, 2021 08:44
@brizental brizental merged commit b6edd65 into mozilla:main Sep 16, 2021
@brizental brizental deleted the 1728810-node-uploader branch September 16, 2021 08:51
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.

2 participants