#6371 get rid of the OK button

[piorek]

Additionally because of code refactor it contains fix for:

• heap size must be positive #6478 heap size must be positive

[scottdraves]

wouldn't it be better to use _.debounce instead of implementing the same idea ourselves?

[piorek]

@scottdraves sorry i missed this comment

[scottdraves]

please move the sync indicator to the bottom, where the "Result" string is, and where the errors are shown.

[scottdraves]

thanks that's better.

1. changes are not synced in all cases. for example if i add a property, and then start typing into the property name, i can see the result changing at the bottom, but sync is not run.

2. when changes are synced, the focus is lost. so if i am typing a property value, and i pause, then its synced (good) but i can't continue typing.

3. also, when in the cursor is in the property name field, and i press TAB, focus should move to the value for that property.

[piorek]

Moved sync indicator

ad 2. and 3.
fixed

ad 1.
i had to disable syncing in that case until there is at least one character in value
because response from backend for payload with empty properties is not containing them

for egzample sending { a1: '1', b1: ''} gives { a1: 1 }

[scottdraves]

thanks. after #6511 is fixed then we should be able to remove "rebuilding" the form, so focus is not lost.

[scottdraves]

OK please bring up to date with master and continue.

[piorek]

Done

[scottdraves]

Great, this is very close.

the remaining problem: if the heap size field is bad, then it should not be sent to server.
right now i can enter "abc" into the heap size, and i see the error reported in the UI.
but the value is still stored in ~/.jupyter/beakerx.json and will crash the server next time i create a notebook:

[E 10:55:52.015 NotebookApp] Failed to load kernel spec: 'clojure'
    Traceback (most recent call last):
      File "/Users/spot/anaconda/envs/beakerx/lib/python3.6/site-packages/notebook/services/kernelspecs/handlers.py", line 57, in get
        d = kernelspec_model(self, kernel_name)
      File "/Users/spot/anaconda/envs/beakerx/lib/python3.6/site-packages/notebook/services/kernelspecs/handlers.py", line 22, in kernelspec_model
        spec = ksm.get_kernel_spec(name)
      File "/Users/spot/anaconda/envs/beakerx/lib/python3.6/site-packages/jupyter_client/kernelspec.py", line 205, in get_kernel_spec
        return self._get_kernel_spec_by_name(kernel_name, resource_dir)
      File "/Users/spot/anaconda/envs/beakerx/lib/python3.6/site-packages/jupyter_client/kernelspec.py", line 192, in _get_kernel_spec_by_name
        return self.kernel_spec_class.from_resource_dir(resource_dir)
      File "/Users/spot/anaconda/envs/beakerx/lib/python3.6/site-packages/jupyter_client/kernelspec.py", line 41, in from_resource_dir
	return cls(resource_dir=resource_dir, **kernel_dict)
      File "/Users/spot/src/beakerx/beakerx/beakerx/kernel_spec.py", line 23, in __init__
        args = EnvironmentSettings.read_beakerx_env_settings()
      File "/Users/spot/src/beakerx/beakerx/beakerx/environment.py", line 95, in read_beakerx_env_settings
	val = float(jvm_settings['heap_GB'])
    ValueError: could not convert string to float: 'abc'

[piorek]

Should be OK now

[scottdraves]

this is broken, please do not change how valid numbers are identified, it's actually quite tricky.
parseFloat("5a") = 5

[piorek]

I did not change the way it was identified

see:

beakerx/beakerx/beakerx/static/tree.js

Lines 54 to 64 in 27dab19

	var val = $("#heap_GB").val().trim();
	if (val.length > 0) {
	if (!isNaN(val)) {
	if (val % 1 === 0) {
	result += '-Xmx' + val + 'g '
	} else {
	result += '-Xmx' + parseInt(val * 1024) + 'm '
	}
	}
	}

beakerx/beakerx/beakerx/static/tree.js

Lines 98 to 101 in 27dab19

	if (val.length > 0 && isNaN(val)) {
	$('#errors').append($('<span>').text("Heap Size must be a decimal number."));
	enable_button = false;
	}

All i did with it was a little extraction refactor, because i needed that logic in two different places.

I added additional check

• isFinite(val) will catch "5a"

[scottdraves]

also if i type a bad heap size, i can see the "sync" seems to run, it turns yellow and then goes green.
it shows a red error and the value in the config does not change, but why did the sync animation run?

[piorek]

"sync" issue you mentioned is fixed now

Should there be some logic to prevent sending valid large numbers like "2e64"?

[scottdraves]

i merged this by mistake and then reverted it.
there must have been an incorrect merge with master because now if i run this branch and enter "5" for the heap size i get a prefs file like this:

{
    "beakerx": {
        "heap_GB": "",
        "jvm_options": {
            "heap_GB": "5",
            "other": [],
            "properties": []
        },
        "version": 2
    }
}

it should be like this:

{
    "beakerx": {
        "jvm_options": {
            "heap_GB": 5,
            "other": [],
            "properties": []
        },
        "version": 2
    }
}

please fix this and make a new PR.

Should there be some logic to prevent sending valid large numbers like "2e64"?

no

[piorek]

Fixed and created new PR #6614