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

VIDEO-8647 - Fixing Chrome Docker Image #1718

Merged
merged 8 commits into from
Feb 28, 2022

Conversation

PikaJoyce
Copy link
Contributor

JIRA Ticket : VIDEO-8647

This PR addresses our Circle CI Job that builds our chrome images. Previously our job was not updating chrome to the latest stable version. We have updated to pulling from cimg/node instead of circleci/node as that is now deprecated. I've added updates to beta and unstable chrome Dockerfile as well.

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

Burndown

Before review

  • Updated CHANGELOG.md if necessary
  • Added unit tests if necessary
  • Updated affected documentation
  • Verified locally with npm test
  • Manually sanity tested running locally
  • Included screenshot as PR comment (if needed)
  • Ready for review

@@ -1,5 +1,5 @@
# https://hub.docker.com/r/browserless/chrome/dockerfile/
FROM circleci/node:lts-browsers
FROM cimg/node:lts
Copy link
Contributor

Choose a reason for hiding this comment

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

you are using cimg/node:current in other Dockerfile and , cimg/node:lts here - why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@makarandp0 The other ones are on beta / unstable and @charliesantos and I were looking at the image tags from circleci and decided that it makes sense for stable to be the current stable node version which is 16.14.0, while "current" would be the latest features 17.6.0.

@makarandp0
Copy link
Contributor

should make same change for firefox images?

@PikaJoyce PikaJoyce requested a review from makarandp0 February 25, 2022 04:54
Copy link
Contributor

@makarandp0 makarandp0 left a comment

Choose a reason for hiding this comment

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

@PikaJoyce, Please revert config.yml changes and update base branch to master.

@@ -1,4 +1,4 @@
FROM circleci/node:latest
FROM cimg/node:current-browsers
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to double check, what is the version of node in this image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

17.6.0 is the current version.

Copy link
Collaborator

@charliesantos charliesantos left a comment

Choose a reason for hiding this comment

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

Please try the integration tests prior merging.

@PikaJoyce PikaJoyce merged commit 67eb0bc into develop Feb 28, 2022
@PikaJoyce PikaJoyce deleted the VIDEO-8647-Fix-Chrome-Docker-Image branch February 28, 2022 20:01
PikaJoyce added a commit that referenced this pull request Feb 28, 2022
PikaJoyce added a commit that referenced this pull request Feb 28, 2022
* VIDEO-8647 Fixing docker images build for Chrome
PikaJoyce added a commit that referenced this pull request Feb 28, 2022
* VIDEO-8647 Fixing docker images build for Chrome
PikaJoyce added a commit that referenced this pull request Mar 8, 2022
* VIDEO-8647 Fixing docker images build for Chrome
PikaJoyce added a commit that referenced this pull request Mar 8, 2022
* Merge Preflight and PlanB removal feature branch (#1702)

* VIDEO-7728 | Better Preflight Errors (#1689)

* Adding better errors on preflight

* lint

* Update docs

* Changelog

* Update docs

* Convert error to string

* Adding timestamp to progress events

* Properly raising signaling errors

* lint

* Update tests

* Mak's feedback

* Update test

* Feature/remove plan b (#1697)

* VIDEO-6587 | Remove Plan B in SDK (#1656)

* Initial implementation.

* Fixing integration tests

* Updating unit tests

* Adding back unit test job

* Removing unneeded case

* Fix build

* Adding rename suggestion

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding changelog

* Update CHANGELOG.md

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding ticket number

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>

* Move changelog to 2.20.0 section

* 2.20.0-rc1

* 2.20.0-dev

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Fix unit test from merge

* 2.20.0-rc2

* 2.20.0-dev

* Update doc

* Update changelog

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Removing framework tests (#1700)

* Convert createLocalTracks to TS (#1694)

* VIDEO-7714 - Convert createLocalTracks to TypeScript

Co-authored-by: joma <[email protected]>
Co-authored-by: charliesantos <[email protected]>

* Update @twilio/webrtc to 4.6.0-rc1 (#1705)

* Update @twilio-webrtc to RC

Co-authored-by: joma <[email protected]>

* 2.21.0-rc1

* 2.21.0-dev

* merge from master

* VIDEO-8791 - Adding extra information into CHANGELOG (#1716)

* Adding extra information into CHANGELOG

* 2.21.0-rc2

* 2.21.0-dev

* VIDEO-8647 - Fixing Chrome Docker Image (#1718)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8954: Adding in extra links and details to the CHANGELOG entry (#1722)

* VIDEO-8954 - Adding in extra links and details to the CHANGELOG entry

* VIDEO-8647 - Fixing Chrome Docker Image (#1718) (#1721) (#1728)

* VIDEO-8647 Fixing docker images build for Chrome

* prep 2.21.0 release

Co-authored-by: Charlemagne Santos <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>
Co-authored-by: Makarand Patwardhan <[email protected]>
PikaJoyce added a commit that referenced this pull request Mar 22, 2022
* Merge Preflight and PlanB removal feature branch (#1702)

* VIDEO-7728 | Better Preflight Errors (#1689)

* Adding better errors on preflight

* lint

* Update docs

* Changelog

* Update docs

* Convert error to string

* Adding timestamp to progress events

* Properly raising signaling errors

* lint

* Update tests

* Mak's feedback

* Update test

* Feature/remove plan b (#1697)

* VIDEO-6587 | Remove Plan B in SDK (#1656)

* Initial implementation.

* Fixing integration tests

* Updating unit tests

* Adding back unit test job

* Removing unneeded case

* Fix build

* Adding rename suggestion

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding changelog

* Update CHANGELOG.md

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding ticket number

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>

* Move changelog to 2.20.0 section

* 2.20.0-rc1

* 2.20.0-dev

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Fix unit test from merge

* 2.20.0-rc2

* 2.20.0-dev

* Update doc

* Update changelog

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Removing framework tests (#1700)

* Convert createLocalTracks to TS (#1694)

* VIDEO-7714 - Convert createLocalTracks to TypeScript

Co-authored-by: joma <[email protected]>
Co-authored-by: charliesantos <[email protected]>

* Update @twilio/webrtc to 4.6.0-rc1 (#1705)

* Update @twilio-webrtc to RC

Co-authored-by: joma <[email protected]>

* 2.21.0-rc1

* 2.21.0-dev

* merge from master

* VIDEO-8791 - Adding extra information into CHANGELOG (#1716)

* Adding extra information into CHANGELOG

* 2.21.0-rc2

* 2.21.0-dev

* VIDEO-8647 - Fixing Chrome Docker Image (#1718)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8954: Adding in extra links and details to the CHANGELOG entry (#1722)

* VIDEO-8954 - Adding in extra links and details to the CHANGELOG entry

* VIDEO-8647 - Fixing Chrome Docker Image (#1718) (#1721) (#1728)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8939 - Migrating twilio/webrtc (#1731)

* VIDEO-8939 - Folding twilio/webrtc into SDK, installing util and events packages.

* Prep for 2.21.1 RC (#1739)

* Adding CHANGELOG entry for 2.21.1 RC

* 2.21.1-rc1

* 2.21.1-dev

* Edit changelog and readme for 2.21.1 release

Co-authored-by: Charlemagne Santos <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>
Co-authored-by: Makarand Patwardhan <[email protected]>
PikaJoyce added a commit that referenced this pull request Jun 1, 2022
* Merge Preflight and PlanB removal feature branch (#1702)

* VIDEO-7728 | Better Preflight Errors (#1689)

* Adding better errors on preflight

* lint

* Update docs

* Changelog

* Update docs

* Convert error to string

* Adding timestamp to progress events

* Properly raising signaling errors

* lint

* Update tests

* Mak's feedback

* Update test

* Feature/remove plan b (#1697)

* VIDEO-6587 | Remove Plan B in SDK (#1656)

* Initial implementation.

* Fixing integration tests

* Updating unit tests

* Adding back unit test job

* Removing unneeded case

* Fix build

* Adding rename suggestion

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding changelog

* Update CHANGELOG.md

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding ticket number

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>

* Move changelog to 2.20.0 section

* 2.20.0-rc1

* 2.20.0-dev

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Fix unit test from merge

* 2.20.0-rc2

* 2.20.0-dev

* Update doc

* Update changelog

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Removing framework tests (#1700)

* Convert createLocalTracks to TS (#1694)

* VIDEO-7714 - Convert createLocalTracks to TypeScript

Co-authored-by: joma <[email protected]>
Co-authored-by: charliesantos <[email protected]>

* Update @twilio/webrtc to 4.6.0-rc1 (#1705)

* Update @twilio-webrtc to RC

Co-authored-by: joma <[email protected]>

* 2.21.0-rc1

* 2.21.0-dev

* merge from master

* VIDEO-8791 - Adding extra information into CHANGELOG (#1716)

* Adding extra information into CHANGELOG

* 2.21.0-rc2

* 2.21.0-dev

* VIDEO-8647 - Fixing Chrome Docker Image (#1718)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8954: Adding in extra links and details to the CHANGELOG entry (#1722)

* VIDEO-8954 - Adding in extra links and details to the CHANGELOG entry

* VIDEO-8647 - Fixing Chrome Docker Image (#1718) (#1721) (#1728)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8939 - Migrating twilio/webrtc (#1731)

* VIDEO-8939 - Folding twilio/webrtc into SDK, installing util and events packages.

* Prep for 2.21.1 RC (#1739)

* Adding CHANGELOG entry for 2.21.1 RC

* 2.21.1-rc1

* 2.21.1-dev

* Marking unstable integration tests (#1740)

* Marking unstable integration tests

* merge from master (#1745)

* VIDEO-9511 - do not configure encodings for stopped tracks (#1768)

* do not update encodings for stopped tracks

* Emit dimensionsChanged event on videoTrack start

* Add test for videoTrack _start method

* Add changelog entry for VIDEO-3576

* VIDEO-9282 - Migrate SDK from using node.js util. (#1752)

* VIDEO-9282 - Removing all references to node dependencies.

Co-authored-by: Makarand Patwardhan <[email protected]>

* 2.21.2-rc1

* 2.21.2-dev

* VIDEO-8282 - Adding iPad Detection (#1779)

* VIDEO-8282 - Adding iPad and iPhone detection logic

* 2.21.2-rc2

* 2.21.2-dev

* Prep for 2.21.2 release, updating changelog

Co-authored-by: Charlemagne Santos <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>
Co-authored-by: Makarand Patwardhan <[email protected]>
Co-authored-by: Tim Mendoza <[email protected]>
Co-authored-by: timmydoza <[email protected]>
charliesantos added a commit that referenced this pull request Jun 7, 2022
* Merge Preflight and PlanB removal feature branch (#1702)

* VIDEO-7728 | Better Preflight Errors (#1689)

* Adding better errors on preflight

* lint

* Update docs

* Changelog

* Update docs

* Convert error to string

* Adding timestamp to progress events

* Properly raising signaling errors

* lint

* Update tests

* Mak's feedback

* Update test

* Feature/remove plan b (#1697)

* VIDEO-6587 | Remove Plan B in SDK (#1656)

* Initial implementation.

* Fixing integration tests

* Updating unit tests

* Adding back unit test job

* Removing unneeded case

* Fix build

* Adding rename suggestion

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding changelog

* Update CHANGELOG.md

Co-authored-by: Manjesh Malavalli <[email protected]>

* Adding ticket number

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>

* Move changelog to 2.20.0 section

* 2.20.0-rc1

* 2.20.0-dev

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Fix unit test from merge

* 2.20.0-rc2

* 2.20.0-dev

* Update doc

* Update changelog

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>

* Removing framework tests (#1700)

* Convert createLocalTracks to TS (#1694)

* VIDEO-7714 - Convert createLocalTracks to TypeScript

Co-authored-by: joma <[email protected]>
Co-authored-by: charliesantos <[email protected]>

* Update @twilio/webrtc to 4.6.0-rc1 (#1705)

* Update @twilio-webrtc to RC

Co-authored-by: joma <[email protected]>

* 2.21.0-rc1

* 2.21.0-dev

* merge from master

* VIDEO-8791 - Adding extra information into CHANGELOG (#1716)

* Adding extra information into CHANGELOG

* 2.21.0-rc2

* 2.21.0-dev

* VIDEO-8647 - Fixing Chrome Docker Image (#1718)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8954: Adding in extra links and details to the CHANGELOG entry (#1722)

* VIDEO-8954 - Adding in extra links and details to the CHANGELOG entry

* VIDEO-8647 - Fixing Chrome Docker Image (#1718) (#1721) (#1728)

* VIDEO-8647 Fixing docker images build for Chrome

* VIDEO-8939 - Migrating twilio/webrtc (#1731)

* VIDEO-8939 - Folding twilio/webrtc into SDK, installing util and events packages.

* Prep for 2.21.1 RC (#1739)

* Adding CHANGELOG entry for 2.21.1 RC

* 2.21.1-rc1

* 2.21.1-dev

* Marking unstable integration tests (#1740)

* Marking unstable integration tests

* merge from master (#1745)

* VIDEO-9511 - do not configure encodings for stopped tracks (#1768)

* do not update encodings for stopped tracks

* Emit dimensionsChanged event on videoTrack start

* Add test for videoTrack _start method

* Add changelog entry for VIDEO-3576

* VIDEO-9282 - Migrate SDK from using node.js util. (#1752)

* VIDEO-9282 - Removing all references to node dependencies.

Co-authored-by: Makarand Patwardhan <[email protected]>

* 2.21.2-rc1

* 2.21.2-dev

* VIDEO-8282 - Adding iPad Detection (#1779)

* VIDEO-8282 - Adding iPad and iPhone detection logic

* 2.21.2-rc2

* 2.21.2-dev

* Added details in changelog entry and updated insights payload constants (#1789)

* 2.21.2-rc3

* 2.21.2-dev

* Merge from master, resolving conflicts, adding new changelog entry (#1796)

* VIDEO-10199 Update tooling back to working versions (#1798)

* Adding platformName into insights payload (#1799)

* Changelog for RC

* 2.21.3-rc1

* 2.21.3-dev

* Update changelog and readme

Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: Manjesh Malavalli <[email protected]>
Co-authored-by: twilio-ci <[email protected]>
Co-authored-by: Joyce Ma <[email protected]>
Co-authored-by: joma <[email protected]>
Co-authored-by: Makarand Patwardhan <[email protected]>
Co-authored-by: Tim Mendoza <[email protected]>
Co-authored-by: timmydoza <[email protected]>
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.

3 participants