Skip to content

Commit

Permalink
matlab-proxy now warns instead of erroring out when Xvfb is not found…
Browse files Browse the repository at this point in the history
… on Linux Systems.

* Makes Xvfb optional on Linux.
* Added infrastructure to display warnings on the status information modal.

fixes mathworks/jupyter-matlab-proxy#75
  • Loading branch information
diningPhilosopher64 authored and Prabhakar Kumar committed Jan 11, 2024
1 parent d92b461 commit 35e01b0
Show file tree
Hide file tree
Showing 11 changed files with 199 additions and 59 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down
2 changes: 1 addition & 1 deletion gui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
20 changes: 11 additions & 9 deletions gui/src/components/Information/Information.css
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -156,4 +158,4 @@
padding-top: 5px;
padding-bottom: 0.3em;
max-width: 10ch;
}
}
36 changes: 31 additions & 5 deletions gui/src/components/Information/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Linkify from 'react-linkify';
import {
selectLicensingInfo,
selectError,
selectWarnings,
selectOverlayHidable,
selectInformationDetails,
selectAuthEnabled,
Expand All @@ -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('');
Expand All @@ -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) {
Expand Down Expand Up @@ -73,8 +73,8 @@ function Information({

const errorLogsNode = (error && error.logs !== null && error.logs.length > 0) ? (
<div className="expand_collapse error-logs-container">
<h4 className={`expand_trigger ${errorLogsExpanded ? 'expanded' : 'collapsed'}`}
onClick={errorLogsExpandedToggle}>
<h4 className={`expand_trigger ${errorLogsExpanded ? 'expanded' : 'collapsed'}`}
onClick={() => setErrorLogsExpanded(!errorLogsExpanded)}>
<span className="icon-arrow-open-down"></span>
<span className="icon-arrow-open-right"></span>
Error logs
Expand All @@ -89,6 +89,31 @@ function Information({
</div>
) : null;

const linkDecorator = (href, text, key) => (
<a href={href} key={key} target="_blank" rel="noopener noreferrer">
{text}
</a>
);


const warningsNode = (warnings && warnings.length > 0) ? (
<div className="expand_collapse warnings-container">
<h4 className={`expand_trigger ${warningsExpanded ? 'expanded' : 'collapsed'}`}
onClick={() => setWarningsExpanded(!warningsExpanded)}>
<span className="icon-arrow-open-down"></span>
<span className="icon-arrow-open-right"></span>
Warnings
</h4>
<div id="warnings"
className={`expand_target warnings-container alert alert-warning ${warningsExpanded ? 'expanded' : 'collapsed'}`}
aria-expanded={warningsExpanded}>
<Linkify componentDecorator={linkDecorator}>
<div className="warnings-msg">{warnings.map((warning, index) => (index + 1).toString() + ")" + warning.trim()).join("\n\n")}</div>
</Linkify>
</div>
</div>
) : null;

const onCloseClick = event => {
if (event.target === event.currentTarget) {
event.preventDefault();
Expand Down Expand Up @@ -197,6 +222,7 @@ function Information({
</div>
{errorMessageNode}
{errorLogsNode}
{warningsNode}
</div>
<div className="modal-footer">
{children}
Expand Down
11 changes: 11 additions & 0 deletions gui/src/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -328,6 +338,7 @@ export default combineReducers({
serverStatus,
loadUrl,
error,
warnings,
envConfig,
useMOS,
useMRE,
Expand Down
1 change: 1 addition & 0 deletions gui/src/selectors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions matlab_proxy/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", ""),
}
)
Expand Down
27 changes: 22 additions & 5 deletions matlab_proxy/app_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
12 changes: 11 additions & 1 deletion matlab_proxy/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}


Expand Down Expand Up @@ -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())
Expand Down
Loading

0 comments on commit 35e01b0

Please sign in to comment.