Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Dockerize Medea #35

Merged
merged 10 commits into from
Jul 18, 2019
Merged

Dockerize Medea #35

merged 10 commits into from
Jul 18, 2019

Conversation

alexlapa
Copy link
Collaborator

@alexlapa alexlapa commented Jul 17, 2019

Part of #34

Solution

  • Dockerize medea
  • Add docker build scripts to makefile
  • Add log level to config

Checklist

  • Created PR:
    • In draft mode
    • Name contains WIP: prefix
    • Name contains issue reference
    • Has k:: labels applied
    • Has assignee
  • Documentation is updated (if required)
  • Tests are updated (if required)
  • Changes conform code style
  • CHANGELOG entry is added (if required)
  • FCM (final commit message) is posted
    • and approved
  • Review is completed and changes are approved
  • Before merge:
    • Milestone is set
    • PR's name and description are correct and up-to-date
    • WIP: prefix is removed
    • All temporary labels are removed

@alexlapa alexlapa added feature New feature or request k::config Related to application configuration k::deploy Related to deployment capabilities labels Jul 17, 2019
@alexlapa alexlapa added this to the 0.1.0 milestone Jul 17, 2019
@alexlapa alexlapa self-assigned this Jul 17, 2019
@alexlapa
Copy link
Collaborator Author

alexlapa commented Jul 17, 2019

FCM:

Dockerize Medea (#35, #34)

- add Dockerfile for Medea
- add docker build script to Makefile

Additionally:
- add log level to config
- up rand to 0.7, slog to 2.5

@alexlapa alexlapa requested a review from tyranron July 17, 2019 01:37
@evdokimovs
Copy link
Contributor

evdokimovs commented Jul 17, 2019

@alexlapa ,

Я тут заметил, что Вы обновили Cargo.lock файл. А у нас с новой версией wasm-bingen перестал работать jason. При открытии e2e-demo в браузере в логах пишет ReferenceError: BigUint64Array is not defined. Я у себя на бренче тоже встречался с этой проблемой, нужно в Cargo.toml'е для jason'а прописать wasm-bindgen = { version = "=0.2.45", features = ["serde-serialize"] }.

@alexlapa
Copy link
Collaborator Author

@evdokimovs ,

Не подтверждаю.

У Вас это на текущей ветке воспроизводится? В каком браузере? А если все предварительно почистить?

rm -rf target
rm -rf /.cargo/git
rm -rf /.cargo/registry
cd jason
wasm-pack build
cd ..
cargo build --bin medea
make up

@alexlapa
Copy link
Collaborator Author

@evdokimovs ,

Вижу его только в сгенерированном jason.js:120

const uint64CvtShim = new BigUint64Array(u32CvtShim.buffer);

А если Вы вручную напишите в консоли: new BigUint64Array([BigInt('1'), BigInt('2')])

@evdokimovs
Copy link
Contributor

evdokimovs commented Jul 17, 2019

@alexlapa ,

У Вас это на текущей ветке воспроизводится?

Проверял на Вашей, проблема есть.

В каком браузере?

В Firefox (v67.0.4) падает просто с ошибкой ReferenceError: BigUint64Array is not defined (на BigUint64Array.prototype тоже самое). А вот в Chrome (70.0.3538.110 (Official Build) ) сейчас проверил, и там все интересней. Вот полный лог:

[WDS] Live Reloading enabled.
index.js?ee1c:46 responder got new connection with member 1
2jason.js?e74a:11 wasm-bindgen: imported JS function that was not marked as `catch` threw an error: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'. 'unified-plan' will become the default behavior in the future, but it is currently experimental. To try it out, construct the RTCPeerConnection with sdpSemantics:'unified-plan' present in the RTCConfiguration argument.

Stack:
Error: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'. 'unified-plan' will become the default behavior in the future, but it is currently experimental. To try it out, construct the RTCPeerConnection with sdpSemantics:'unified-plan' present in the RTCConfiguration argument.
    at Module.__widl_f_add_transceiver_with_str_and_init_RTCPeerConnection (webpack-internal:///../pkg/jason.js:899:37)
    at __widl_f_add_transceiver_with_str_and_init_RTCPeerConnection (http://localhost:8081/bundle.js:140:130)
    at web_sys::RtcPeerConnection::add_transceiver_with_str_and_init::h011e9e0092976e5c (wasm-function[2527]:191)
    at jason::peer::conn::RtcPeerConnection::add_transceiver::hedfb42170325b94b (wasm-function[1263]:352)
    at jason::peer::media::Sender::new::ha27c0139e5c52fd4 (wasm-function[299]:337)
    at jason::peer::media::MediaConnections::update_tracks::h10cf2419b706967d (wasm-function[109]:3156)
    at jason::peer::PeerConnection::get_offer::hddb2f2e762b1fe24 (wasm-function[221]:250)
    at <jason::api::room::InnerRoom as medea_client_api_proto::EventHandler>::on_peer_created::h763f147e312c0592 (wasm-function[247]:749)
    at medea_client_api_proto::Event::dispatch_with::hef42ebda74bae1a6 (wasm-function[258]:667)
    at jason::api::room::Room::new::{{closure}}::hca08b3c648d05c34 (wasm-function[552]:523)
logError @ jason.js?e74a:11
localhost/:1 Uncaught (in promise) DOMException: Failed to execute 'addTransceiver' on 'RTCPeerConnection': This operation is only supported in 'unified-plan'. 'unified-plan' will become the default behavior in the future, but it is currently experimental. To try it out, construct the RTCPeerConnection with sdpSemantics:'unified-plan' present in the RTCConfiguration argument.
    at Module.__widl_f_add_transceiver_with_str_and_init_RTCPeerConnection (webpack-internal:///../pkg/jason.js:899:37)
    at __widl_f_add_transceiver_with_str_and_init_RTCPeerConnection (http://localhost:8081/bundle.js:140:130)
    at web_sys::RtcPeerConnection::add_transceiver_with_str_and_init::h011e9e0092976e5c (wasm-function[2527]:191)
    at jason::peer::conn::RtcPeerConnection::add_transceiver::hedfb42170325b94b (wasm-function[1263]:352)
    at jason::peer::media::Sender::new::ha27c0139e5c52fd4 (wasm-function[299]:337)
    at jason::peer::media::MediaConnections::update_tracks::h10cf2419b706967d (wasm-function[109]:3156)
    at jason::peer::PeerConnection::get_offer::hddb2f2e762b1fe24 (wasm-function[221]:250)
    at <jason::api::room::InnerRoom as medea_client_api_proto::EventHandler>::on_peer_created::h763f147e312c0592 (wasm-function[247]:749)
    at medea_client_api_proto::Event::dispatch_with::hef42ebda74bae1a6 (wasm-function[258]:667)
    at jason::api::room::Room::new::{{closure}}::hca08b3c648d05c34 (wasm-function[552]:523)

Вот эта ошибка с Chrome оказалась все таки еще интерсней, чем я думал. Она никак не связана с wasm-bindgen, потому-что выпадает и на моем бренче с поправленной версией.

А если все предварительно почистить?

Ошибка не уходит.

А если Вы вручную напишите в консоли: new BigUint64Array([BigInt('1'), BigInt('2')])

ReferenceError: BigUint64Array is not defined.

@alexlapa
Copy link
Collaborator Author

Дело в версиях бразуеров. unified-plan в хроме добавили в версии 72, BigUint64Array в FF добавили в версии 68.

Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

Refactored a bit, and also has setup Docker Hub image.

FCM:

Dockerize Medea (#35, #34)

- impl instrumentisto/medea Docker image
- add docker.build Makefile command

Additionally:
- add logging settings to config
- upd 'rand' to 0.7 and 'slog' to 2.5

Please, be careful when specifying FCM to merge commit form.

Dockerfile Outdated
&& CARGO_HOME="${cargo_home}" \
# TODO: use --out-dir once stabilized
# TODO: https://github.com/rust-lang/cargo/issues/6790
cargo build --offline --bin=medea ${rustc_opts} \
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, we cannot use --offline. The image should be able to build without Makefile at all. That's exactly what would happen on Docker Hub.

Makefile here only gives us "tricks" to reduce image build time during dev/CI.

$(call docker.build.clean.ignore)
define docker.build.clean.ignore
@sed -i $(if $(call eq,$(shell uname -s),Darwin),'',) \
/^!target\/d .dockerignore
Copy link
Member

Choose a reason for hiding this comment

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

I've refactored this to support macOS.

@alexlapa alexlapa changed the title WIP: Dockerize Medea Dockerize Medea Jul 18, 2019
@alexlapa alexlapa merged commit 0721706 into master Jul 18, 2019
@tyranron tyranron deleted the 34-dockerize-medea branch July 18, 2019 11:12
@tyranron tyranron added the enhancement Improvement of existing features or bugfix label Jul 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of existing features or bugfix feature New feature or request k::config Related to application configuration k::deploy Related to deployment capabilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants