Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

add_instance_to_interface improvement: sending a response via websockets after just shutting down/restarting doesn't work #2172

Open
Connoropolous opened this issue Mar 28, 2020 · 3 comments

Comments

@Connoropolous
Copy link
Collaborator

Connoropolous commented Mar 28, 2020

self.io
.add_method("admin/interface/add_instance", move |params| {
let params_map = Self::unwrap_params_map(params)?;
let interface_id = Self::get_as_string("interface_id", &params_map)?;
let instance_id = Self::get_as_string("instance_id", &params_map)?;
let alias = Self::get_as_string("alias", &params_map).ok();
conductor_call!(|c| c.add_instance_to_interface(
&interface_id,
&instance_id,
&alias
))?;
Ok(json!({"success": true}))
});

add_instance_to_interface causes clients of the interface who called this function to lose their connection, and thus lose their response to their call. I wonder whether you could add_instance_to_interface, send response, then restart interface outside of add_instance_to_interface instead?

for context, I am dealing with this here, and you will notice I have to do some pretty major workarounding
https://github.com/h-be/acorn-ui/blob/master/src/routes/Dashboard/Dashboard.js#L195-L228

This is where, within add_instance_to_interface, it restarts the interface

let _ = self.stop_interface_by_id(interface_id);
sleep(Duration::from_millis(SWEET_SLEEP));
self.start_interface_by_id(interface_id)?;

@Connoropolous Connoropolous changed the title sending a response via websockets after just shutting down/restarting doesn't work add_instance_to_interface improvement: sending a response via websockets after just shutting down/restarting doesn't work Mar 28, 2020
@Connoropolous
Copy link
Collaborator Author

Connoropolous commented Mar 29, 2020

I think Holoscape works around this by just refreshing a whole browser client page, but that is a very non-ideal solution

@StaticallyTypedAnxiety
Copy link
Contributor

Wondering why the interface has to be restarted anywway,

  • If the interface is already exists, it will now hit the restart subsection because of the return error
  • is there a reason why an already running instance should be restarted? to apply changes maybe?

@Connoropolous
Copy link
Collaborator Author

@AshantiMutinta thanks :)

I am looking closer into it. The issue, (why it needs to restart the interface), is because the interface JSON-RPC calls are statically defined during interface start up, versus dynamically checked during JSON-RPC calls...
see here, traced manually along this call stack...

self.start_interface_by_id(interface_id)?;

.and_then(|config| self.start_interface(&config))

let handle = self.spawn_interface_thread(config.clone());

let dispatcher = self.make_interface_handler(&interface_config);

conductor_api_builder.spawn()


fn setup_call_api(&mut self) {
let instances = self.instances.clone();
let instance_ids_map = self.instance_ids_map.clone();
self.io.add_method("call", move |params| {
let instances = instances.clone();
let instance_ids_map = instance_ids_map.clone();
let response = Self::method_call(params, instances, instance_ids_map)?;
Ok(Value::String(response.to_string()))
});
}

Notice how the instances are stored as a hard reference outside of the JSON-RPC call.
Those assumptions would need to be re-wired differently in order to prevent the stopping and restarting of the interface

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants