diff --git a/README.md b/README.md
index e5d9e44b..9da9828d 100644
--- a/README.md
+++ b/README.md
@@ -47,9 +47,10 @@ The MATLAB Proxy is under active development. For support or to report issues, s
$ sudo yum install xorg-x11-server-Xvfb
```
+
+ *Note: The installation of Xvfb is **optional** (w.e.f. v0.11.0 of matlab-proxy). However, we highly recommend installing it.*
* Python versions: **3.8** | **3.9** | **3.10** | **3.11**
* [Browser Requirements](https://www.mathworks.com/support/requirements/browser-requirements.html)
-
* Supported Operating Systems:
* Linux®
* Windows® Operating System ( starting v0.4.0 of matlab-proxy )
diff --git a/gui/package.json b/gui/package.json
index e69898c8..b9709f97 100644
--- a/gui/package.json
+++ b/gui/package.json
@@ -40,7 +40,7 @@
"last 1 safari version"
]
},
- "proxy": "http://127.0.0.1:8000",
+ "proxy": "http://127.0.0.1:8888",
"homepage": "./",
"devDependencies": {
"@babel/preset-env": "^7.11.0",
diff --git a/gui/src/components/Information/Information.css b/gui/src/components/Information/Information.css
index 6ca28b4b..e111150d 100644
--- a/gui/src/components/Information/Information.css
+++ b/gui/src/components/Information/Information.css
@@ -1,6 +1,6 @@
/* Copyright (c) 2020-2022 The MathWorks, Inc. */
-#information .alert:not(.error-container) {
+#information .alert:not(.error-container, .warnings-container) {
padding: 0;
margin-bottom: 0;
background: white;
@@ -24,37 +24,39 @@
margin-bottom: 0;
}
-#information .error-msg {
+#information .error-msg, .warnings-msg {
white-space: pre;
font: 10pt monospace;
}
-#information .error-container.alert {
+#information .error-container.alert, .warnings-container.alert {
border-radius: 0;
margin: 5px 20px;
padding: 10px;
}
-#information .error-container.alert p {
+#information .error-container.alert p, .warnings-container.alert p {
margin: 0;
}
-#information .error-container.alert .error-text {
+#information .error-container.alert .error-text,
+.warnings-container.alert .warnings-text {
margin-left: 30px;
}
-#information .error-logs-container .error-container.alert {
+#information .error-logs-container .error-container.alert,
+.warnings-container .warnings-container.alert {
padding: 0;
/* if this is inside an expand-collapse, padding just gets in the way */
}
-#information .error-msg {
+#information .error-msg, .warnings-msg {
padding: 10px;
max-height: 200px;
overflow: auto;
}
-#information .error-logs-container .expand_trigger {
+#information .error-logs-container .warnings-container .expand_trigger {
padding: 10px 5px 5px 40px;
font-size: 15px;
}
@@ -156,4 +158,4 @@
padding-top: 5px;
padding-bottom: 0.3em;
max-width: 10ch;
-}
\ No newline at end of file
+}
diff --git a/gui/src/components/Information/index.js b/gui/src/components/Information/index.js
index b04e2ddc..250200e8 100644
--- a/gui/src/components/Information/index.js
+++ b/gui/src/components/Information/index.js
@@ -7,6 +7,7 @@ import Linkify from 'react-linkify';
import {
selectLicensingInfo,
selectError,
+ selectWarnings,
selectOverlayHidable,
selectInformationDetails,
selectAuthEnabled,
@@ -22,6 +23,7 @@ function Information({
}) {
const licensingInfo = useSelector(selectLicensingInfo);
const error = useSelector(selectError);
+ const warnings = useSelector(selectWarnings);
const overlayHidable = useSelector(selectOverlayHidable);
const [token, setToken] = useState('');
@@ -33,9 +35,7 @@ function Information({
const tokenInput = useRef();
const [errorLogsExpanded, setErrorLogsExpanded] = useState(false);
- const errorLogsExpandedToggle = () => {
- setErrorLogsExpanded(!errorLogsExpanded);
- };
+ const [warningsExpanded, setWarningsExpanded] = useState(true);
let info;
switch (licensingInfo?.type) {
@@ -73,8 +73,8 @@ function Information({
const errorLogsNode = (error && error.logs !== null && error.logs.length > 0) ? (
-
+ setErrorLogsExpanded(!errorLogsExpanded)}>
Error logs
@@ -89,6 +89,31 @@ function Information({
) : null;
+ const linkDecorator = (href, text, key) => (
+
+ {text}
+
+ );
+
+
+ const warningsNode = (warnings && warnings.length > 0) ? (
+
+
setWarningsExpanded(!warningsExpanded)}>
+
+
+ Warnings
+
+
+
+ {warnings.map((warning, index) => (index + 1).toString() + ")" + warning.trim()).join("\n\n")}
+
+
+
{children}
diff --git a/gui/src/reducers/index.js b/gui/src/reducers/index.js
index 9271b4dc..c86bb121 100644
--- a/gui/src/reducers/index.js
+++ b/gui/src/reducers/index.js
@@ -253,6 +253,16 @@ export function loadUrl(state = null, action) {
}
}
+export function warnings(state = null, action) {
+ switch (action.type) {
+ case RECEIVE_SERVER_STATUS:
+ const warnings = action.status.warnings;
+ return warnings.length > 0 ? warnings : null;
+ default:
+ return state;
+ }
+}
+
export function error(state = null, action) {
switch (action.type) {
case SET_AUTH_STATUS:
@@ -328,6 +338,7 @@ export default combineReducers({
serverStatus,
loadUrl,
error,
+ warnings,
envConfig,
useMOS,
useMRE,
diff --git a/gui/src/selectors/index.js b/gui/src/selectors/index.js
index 9538190c..c0f48d23 100644
--- a/gui/src/selectors/index.js
+++ b/gui/src/selectors/index.js
@@ -15,6 +15,7 @@ export const selectLicensingInfo = state => state.serverStatus.licensingInfo;
export const selectServerStatusFetchFailCount = state => state.serverStatus.fetchFailCount;
export const selectLoadUrl = state => state.loadUrl;
export const selectError = state => state.error;
+export const selectWarnings = state => state.warnings;
export const selectUseMOS = state => state.useMOS === true;
export const selectUseMRE = state => state.useMRE === true;
export const selectAuthEnabled = state => state.authentication.enabled;
diff --git a/matlab_proxy/app.py b/matlab_proxy/app.py
index 2ed929d4..94c36b9e 100644
--- a/matlab_proxy/app.py
+++ b/matlab_proxy/app.py
@@ -113,6 +113,7 @@ async def create_status_response(app, loadUrl=None):
"licensing": marshal_licensing_info(state.licensing),
"loadUrl": loadUrl,
"error": marshal_error(state.error),
+ "warnings": state.warnings,
"wsEnv": state.settings.get("ws_env", ""),
}
)
diff --git a/matlab_proxy/app_state.py b/matlab_proxy/app_state.py
index 2ce9bcd4..711fdd1c 100644
--- a/matlab_proxy/app_state.py
+++ b/matlab_proxy/app_state.py
@@ -84,6 +84,7 @@ def __init__(self, settings):
# Initialize with the error state from the initialization of settings
self.error = settings["error"]
+ self.warnings = settings["warnings"]
if self.error is not None:
self.logs["matlab"].clear()
@@ -262,7 +263,10 @@ def _are_required_processes_ready(
xvfb_process = self.processes["xvfb"]
if system.is_linux():
- if xvfb_process is None or xvfb_process.returncode is not None:
+ # If Xvfb is on system PATH, check if it up and running.
+ if self.settings["is_xvfb_available"] and (
+ xvfb_process is None or xvfb_process.returncode is not None
+ ):
logger.debug(
"Xvfb has not started"
if xvfb_process is None
@@ -671,9 +675,22 @@ async def __setup_env_for_matlab(self) -> dict:
# DDUX info for MATLAB
matlab_env["MW_CONTEXT_TAGS"] = self.settings.get("mw_context_tags")
+ # Update DISPLAY env variable for MATLAB only if it was supplied by Xvfb.
if system.is_linux():
- # Adding DISPLAY key which is only available after starting Xvfb successfully.
- matlab_env["DISPLAY"] = self.settings["matlab_display"]
+ if self.settings.get("matlab_display", None):
+ matlab_env["DISPLAY"] = self.settings["matlab_display"]
+ logger.info(
+ f"Using the display number supplied by Xvfb process:{matlab_env['DISPLAY']} for launching MATLAB"
+ )
+ else:
+ if "DISPLAY" in matlab_env:
+ logger.info(
+ f"Using the existing DISPLAY environment variable with value:{matlab_env['DISPLAY']} for launching MATLAB"
+ )
+ else:
+ logger.info(
+ "No DISPLAY environment variable found. Launching MATLAB without it."
+ )
# The matlab ready file is written into this location(self.mwi_logs_dir) by MATLAB
# The mwi_logs_dir is where MATLAB will write any subsequent logs
@@ -968,8 +985,8 @@ async def start_matlab(self, restart_matlab=False):
self.error = None
self.logs["matlab"].clear()
- # Start Xvfb process if in a posix system
- if system.is_linux():
+ # Start Xvfb process on linux if possible
+ if system.is_linux() and self.settings["is_xvfb_available"]:
xvfb = await self.__start_xvfb_process()
# xvfb variable would be None if creation of the process failed.
diff --git a/matlab_proxy/settings.py b/matlab_proxy/settings.py
index 62b6a497..57bf9c34 100644
--- a/matlab_proxy/settings.py
+++ b/matlab_proxy/settings.py
@@ -199,6 +199,8 @@ def get_dev_settings(config):
"mwi_use_existing_license": mwi.validators.validate_use_existing_licensing(
os.getenv(mwi_env.get_env_name_mwi_use_existing_license(), "")
),
+ "warnings": [],
+ "is_xvfb_available": False,
}
@@ -243,13 +245,21 @@ def get(config_name=matlab_proxy.get_default_config_name(), dev=False):
settings["nlm_conn_str"] = "123@nlm"
else:
- settings = {"error": None}
+ settings = {"error": None, "warnings": []}
# Initializing server settings separately allows us to return
# a minimal set of settings required to launch the server even if
# there is an exception thrown when creating the matlab specific settings.
settings.update(get_server_settings(config_name))
+ settings["is_xvfb_available"] = True if shutil.which("Xvfb") else False
+
+ # Warn user if xvfb is not available on system path.
+ if system.is_linux() and not settings["is_xvfb_available"]:
+ warning = " Unable to find Xvfb on the system PATH. Xvfb enables graphical abilities like plots and figures in the MATLAB desktop.\nConsider adding Xvfb to the system PATH and restart matlab-proxy. See https://github.com/mathworks/matlab-proxy#requirements for information."
+ logger.warning(warning)
+ settings["warnings"].append(warning)
+
try:
# Update settings with matlab specific values.
settings.update(get_matlab_settings())
diff --git a/tests/unit/test_app_state.py b/tests/unit/test_app_state.py
index a3116b7b..e82c7760 100644
--- a/tests/unit/test_app_state.py
+++ b/tests/unit/test_app_state.py
@@ -7,6 +7,7 @@
from typing import Optional
import pytest
+from matlab_proxy import settings
from matlab_proxy.app_state import AppState
from matlab_proxy.util.mwi.exceptions import LicensingError, MatlabError
@@ -14,14 +15,41 @@
@pytest.fixture
-def app_state_fixture():
+def sample_settings_fixture(tmp_path):
+ """A pytest fixture which returns a dict containing sample settings for the AppState class.
+
+ Args:
+ tmp_path : Builtin pytest fixture
+
+ Returns:
+ dict: A dictionary of sample settings
+ """
+ tmp_file = tmp_path / "parent_1" / "parent_2" / "tmp_file.json"
+ return {
+ "error": None,
+ "warnings": [],
+ "matlab_config_file": tmp_file,
+ "is_xvfb_available": True,
+ "mwi_server_url": "dummy",
+ "mwi_logs_root_dir": Path(settings.get_mwi_config_folder(dev=True)),
+ "app_port": 12345,
+ "mwapikey": "asdf",
+ }
+
+
+@pytest.fixture
+def app_state_fixture(sample_settings_fixture):
"""A pytest fixture which returns an instance of AppState class with no errors.
+ Args:
+ sample_settings_fixture (dict): A dictionary of sample settings to be used by
+
Returns:
AppState: An object of the AppState class
"""
- settings = {"error": None}
- app_state = AppState(settings=settings)
+ app_state = AppState(settings=sample_settings_fixture)
+ app_state.processes = {"matlab": None, "xvfb": None}
+ app_state.licensing = {"type": "existing_license"}
return app_state
@@ -91,6 +119,7 @@ class Mock_xvfb:
"""An immutable dataclass representing a mocked Xvfb process"""
returncode: Optional[int]
+ pid: Optional[int]
@dataclass(frozen=True)
@@ -98,6 +127,7 @@ class Mock_matlab:
"""An immutable dataclass representing a mocked MATLAB process"""
returncode: Optional[int]
+ pid: Optional[int]
@pytest.mark.parametrize(
@@ -156,7 +186,7 @@ def test_is_licensed(app_state_fixture, licensing, expected):
],
ids=["Any error except licensing error", "licensing error"],
)
-def test_unset_licensing(err, expected_err):
+def test_unset_licensing(err, app_state_fixture, expected_err):
"""Test to check unset_liecnsing removes licensing from the AppState object
Args:
@@ -165,34 +195,32 @@ def test_unset_licensing(err, expected_err):
expected_err (Exception): Expected exception
"""
# Arrange
- settings = {"error": err}
- app_state = AppState(settings=settings)
+ app_state_fixture.error = err
# Act
- app_state.unset_licensing()
+ app_state_fixture.unset_licensing()
# Assert
- assert app_state.licensing == None
- assert type(app_state.error) is type(expected_err)
+ assert app_state_fixture.licensing == None
+ assert type(app_state_fixture.error) is type(expected_err)
# config file is deleted when licensing info is not set i.e. set to None
-def test_persist_licensing_when_licensing_info_is_not_set(tmp_path):
+def test_persist_licensing_when_licensing_info_is_not_set(app_state_fixture):
"""Test to check if data is not persisted to a file if licensing info is not present
Args:
tmp_path (Path): Built in pytest fixture
"""
# Arrange
- tmp_file = tmp_path / "tmp_file.json"
- settings = {"matlab_config_file": tmp_file, "error": None}
- app_state = AppState(settings=settings)
+ # Nothing to arrange
+ app_state_fixture.licensing = None
# Act
- app_state.persist_config_data()
+ app_state_fixture.persist_config_data()
# Assert
- assert os.path.exists(tmp_file) is False
+ assert os.path.exists(app_state_fixture.settings["matlab_config_file"]) is False
@pytest.mark.parametrize(
@@ -221,7 +249,12 @@ def test_persist_config_data(licensing_data: dict, tmp_path):
"""
# Arrange
tmp_file = tmp_path / "parent_1" / "parent_2" / "tmp_file.json"
- settings = {"matlab_config_file": tmp_file, "error": None, "matlab_version": None}
+ settings = {
+ "matlab_config_file": tmp_file,
+ "error": None,
+ "matlab_version": None,
+ "warnings": [],
+ }
app_state = AppState(settings=settings)
app_state.licensing = licensing_data
@@ -238,25 +271,31 @@ def test_persist_config_data(licensing_data: dict, tmp_path):
validate_required_processes_test_data = [
(None, None, "linux", False), # xvfb is None == True
- (None, Mock_xvfb(None), "linux", False), # matlab is None == True
+ (None, Mock_xvfb(None, 1), "linux", False), # matlab is None == True
(
- Mock_matlab(None),
- Mock_xvfb(None),
+ Mock_matlab(None, 1),
+ Mock_xvfb(None, 1),
"linux",
True,
), # All branches are skipped and nothing returned
(
- Mock_matlab(None),
- Mock_xvfb(123),
+ Mock_matlab(None, 1),
+ Mock_xvfb(123, 2),
"linux",
False,
), # xvfb.returncode is not None == True
(
- Mock_matlab(123),
- Mock_xvfb(None),
+ Mock_matlab(123, 1),
+ Mock_xvfb(None, 2),
"linux",
False,
), # matlab.returncode is not None == True
+ (
+ Mock_matlab(None, 1),
+ None,
+ "linux",
+ True,
+ ), # Xvfb not found on path
]
@@ -269,6 +308,7 @@ def test_persist_config_data(licensing_data: dict, tmp_path):
"All_required_processes_running",
"All_processes_running_with_xvfb_returning_non_zero_code",
"All_processes_running_with_matlab_returning_non_zero_code",
+ "xvfb_is_optional_matlab_starts_without_it",
],
)
def test_are_required_processes_ready(
@@ -284,10 +324,12 @@ def test_are_required_processes_ready(
expected (bool): Expected return value based on process return code
"""
# Arrange
- # Nothing to arrange
+ app_state_fixture.processes = {"matlab": matlab, "xvfb": xvfb}
+ if not xvfb:
+ app_state_fixture.settings["is_xvfb_available"] = False
# Act
- actual = app_state_fixture._are_required_processes_ready(matlab, xvfb)
+ actual = app_state_fixture._are_required_processes_ready()
# Assert
assert actual == expected
@@ -306,7 +348,7 @@ def test_are_required_processes_ready(
ids=["connector_up", "connector_down", "connector_up_ready_file_not_present"],
)
async def test_get_matlab_status_based_on_connector_status(
- mocker, connector_status, ready_file_present, matlab_status
+ mocker, app_state_fixture, connector_status, ready_file_present, matlab_status
):
"""Test to check matlab status based on connector status
@@ -322,16 +364,11 @@ async def test_get_matlab_status_based_on_connector_status(
return_value=connector_status,
)
mocker.patch.object(Path, "exists", return_value=ready_file_present)
- settings = {
- "error": None,
- "mwi_server_url": "dummy",
- "mwi_is_token_auth_enabled": False,
- }
- app_state = AppState(settings=settings)
- app_state.matlab_session_files["matlab_ready_file"] = Path("dummy")
+ app_state_fixture.settings["mwi_is_token_auth_enabled"] = False
+ app_state_fixture.matlab_session_files["matlab_ready_file"] = Path("dummy")
# Act
- actual_matlab_status = await app_state._get_matlab_connector_status()
+ actual_matlab_status = await app_state_fixture._get_matlab_connector_status()
# Assert
assert actual_matlab_status == matlab_status
@@ -510,3 +547,36 @@ async def test_requests_sent_by_matlab_proxy_have_headers(
"headers"
]
assert sample_token_headers_fixture == connector_status_request_headers
+
+
+async def test_start_matlab_without_xvfb(app_state_fixture, mocker):
+ """Test to check if Matlab process starts without throwing errors when Xvfb is not present
+
+ Args:
+ app_state_fixture (AppState): Object of AppState class with defaults set
+ mocker : Built-in pytest fixture
+ """
+ # Arrange
+ app_state_fixture.settings["is_xvfb_available"] = False
+ mock_matlab = Mock_matlab(None, 1)
+
+ # Starting asyncio tasks related to matlab is not required here as only Xvfb check is required.
+ mocker.patch.object(
+ AppState, "_AppState__start_matlab_process", return_value=mock_matlab
+ )
+ mocker.patch.object(
+ AppState, "_AppState__matlab_stderr_reader_posix", return_value=None
+ )
+ mocker.patch.object(
+ AppState, "_AppState__track_embedded_connector_state", return_value=None
+ )
+ mocker.patch.object(AppState, "_AppState__update_matlab_port", return_value=None)
+
+ # Act
+ await app_state_fixture.start_matlab()
+
+ # Assert
+ # Check if Xvfb has not started
+ assert app_state_fixture.processes["xvfb"] is None
+ # Check if Matlab started
+ assert app_state_fixture.processes["matlab"] is mock_matlab
diff --git a/tests/unit/util/test_mw.py b/tests/unit/util/test_mw.py
index 18b2dcad..95708037 100644
--- a/tests/unit/util/test_mw.py
+++ b/tests/unit/util/test_mw.py
@@ -525,10 +525,11 @@ async def test_create_xvfb_process(loop):
Creates 2 xvfb processes with '-displayfd' flag and checks if the processes are
running on unique display ports
"""
-
- # Get command to launch xvfb with -displayfd flag.
settings_1 = settings.get(dev=True)
- settings_2 = settings.get(dev=True)
+
+ # Return if Xvfb is not available
+ if not settings_1["is_xvfb_available"]:
+ return
xvfb_cmd_1, pipe_1 = settings.create_xvfb_cmd()
xvfb_cmd_2, pipe_2 = settings.create_xvfb_cmd()