himdel
commented
4 years ago Cc @rvsia (app/javascript/components/workers-form/workers-form.jsx)
We probably shouldn't be ignoring the error message from the API.
Closed carbonin closed 4 years ago
himdel
commented
4 years ago Cc @rvsia (app/javascript/components/workers-form/workers-form.jsx)
We probably shouldn't be ignoring the error message from the API.
himdel
commented
4 years ago (Also present in app/javascript/components/catalog-form/catalog-form.jsx)
himdel
commented
4 years ago @carbonin Can you also share the output of /api/servers/1/settings please?
carbonin
commented
4 years ago It's very large ...
carbonin
commented
4 years ago If it's more helpful, these are the only entries in settings_changes:
vmdb_production=# select key,value from settings_changes;
-[ RECORD 1 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /server/host
value | --- 192.168.122.180 +
| ... +
|
-[ RECORD 2 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /server/hostname
value |
-[ RECORD 3 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /server/role
value | --- automate,database_operations,embedded_ansible,ems_inventory,ems_operations,event,remote_console,reporting,scheduler,smartstate,user_interface,web_services+
| ... +
|
-[ RECORD 4 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /server/locale
value | --- default +
| ... +
|
-[ RECORD 5 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /server/zone
value | --- default +
| ... +
|
-[ RECORD 6 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /server/name
value | --- EVM +
| ... +
|
-[ RECORD 7 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /smtp/enable_starttls_auto
value | --- true +
| ... +
|
-[ RECORD 8 ]---------------------------------------------------------------------------------------------------------------------------------------------------------
key | /smtp/openssl_verify_mode
value | --- none +
| ... +
| Thanks, the API response is perfect :)
I think what ends up happening is that there is no error from the API, but an exception in the code parsing the response gets treated as if there were a generic API error with no message.
himdel
commented
4 years ago The actual failure:
TypeError: Cannot destructure property `count` of 'undefined' or 'null'.
at parseWorkerspecifically...
vim_broker_worker: {
memory_threshold: parseWorker(wb.vim_broker_worker).bytes || memDefault,
},fails because wb.vim_broker_worker is undefined.
And the || memDefault logic has no chance of ever working given that.
I don't believe the UI should really be hardcoding the list of workers, or failing when one is not present.
I can certainly fix it so that it doesn't fail on missing workers, but not sure if we have a way of telling what to display for which worker, if we don't hardcode the list. (see https://user-images.githubusercontent.com/32869456/57625031-5f3d3580-7593-11e9-8042-a912e5cda2bc.png and https://github.com/ManageIQ/manageiq-ui-classic/pull/5528/files#diff-9ecdde9748ab4e2ee8f77589321e9ab6R30-R78 )
himdel
commented
4 years ago @carbonin Do you happen to know, is there any system to which workers should expose the "Count" combobox and which ones should expose the "Memory Threshold" one? (screenshot)
And can we rely on all the workers that exist being present in settings? Or is there another way to get the full list?
carbonin
commented
4 years ago I think the problem here is that the vim broker worker was removed. So in response to "And can we rely on all the workers that exist being present in settings?"
I would say yes, but this one actually doesn't exist.
As for if we can make the worker types more dynamic ... we have a WorkerType model that we could expose over the API, but it also looks like you need the settings path for each worker type so that would have to be in the response somewhere.
I assume removing the vim_broker_worker bit fixes this for the short term?
himdel
commented
4 years ago I assume removing the vim_broker_worker bit fixes this for the short term?
Thanks, OK, removing the vim_broker_worker and updating the code to deal a bit better with anything missing then, at least for now :).
But sounds like we should and could expose this over the API, so I'll create an API issue?
carbonin
commented
4 years ago But sounds like we should and could expose this over the API, so I'll create an API issue?
Yeah I think that makes sense. Will need to figure out exactly what needs exposing, but we can do that in the newly opened issue :smile:
production log shows success:
But the screen gives the following error: