snu-quiqcl / qiwis

QuIqcl Widget Integration Software
MIT License
5 stars 2 forks source link

[BUG] Handle re-creating the existing app #190

Closed BECATRUE closed 11 months ago

BECATRUE commented 1 year ago

Describe the bug

When an app A exists, if we requests creating the app A again, it seems to be added well on the GUI, but internally, the connection to the existing app is lost.

Expected behavior

I came up with two solutions:

  1. Ignore the incoming request
  2. Replace the original app into the new one

Additional context

For example, when you try to reopen a builder that is already open, how do you deal with it?

BECATRUE commented 1 year ago

What do you think of this problem? @kangz12345

kangz12345 commented 1 year ago

Hmm, could you explain in more detail? For example:

  1. "An app A" means the class A or the string name "A"?
  2. What happens if the connection is lost? Which features don't work?

For example, when you try to reopen a builder that is already open, how do you deal with it?

If it is for the same experiment, I would expect to be focused to the already opened window.

BECATRUE commented 1 year ago
  1. "An app A" means the class A or the string name "A"?

I intended the string name "A". In createApp(), we use the string name to distinguish apps, so we cannot have multiple apps with the same name.

  1. What happens if the connection is lost? Which features don't work?

The original app is still remaining in GUI, but the qiwis._apps dictionary doesn't contain the app anymore. So we cannot remove the app using destroyApp and it cannot receive messages of the subscribing channels.

If it is for the same experiment, I would expect to be focused to the already opened window.

I also agree with you! Currently in order to implement this, first we reqeust a qiwiscall to fetch the app name list and second compare them directly. But I think it is inconvenient because we use two qiwiscalls; createApp and appNames, so I want to handle it internally.

kangz12345 commented 1 year ago

Thank you for the explanation. In that case, I think createApp() should check if the name already exists, and raise an error if it does.

BECATRUE commented 1 year ago

Oh, I came up with an idea!

As you said, createApp() should check it, but it may raise an error or replace the original app into the new one according to the given option in config.json. I think it helps us to refresh the app through just re-opening it.

But it may be convoluted, so if you think it isn't necessary, it's okay to stick to the above decision.

kangz12345 commented 1 year ago

Hmm, if you think that such feature is helpful for some cases, I think giving it as a parameter would be enough. For example, createApp(name: str, info: AppInfo, replace: bool = False):.

BECATRUE commented 12 months ago

Oh, it looks simpler! I also agree with you.

kangz12345 commented 11 months ago

You linked the wrong issue there. I will close this as completed.

cf. You can search issue after typing #. For example, #re-creating...