Update least_busy #2323
Update least_busy #2323
Conversation
| self._backends_list = self._active_api_client.list_backends() | ||
| if not self._backends_list: | ||
| self._backends_list = self._active_api_client.list_backends() |
There was a problem hiding this comment.
if list_backends is going to be called anyways outside of the if-else, why not do:
| self._backends_list = self._active_api_client.list_backends() | |
| if not self._backends_list: | |
| self._backends_list = self._active_api_client.list_backends() | |
| if not self._backends_list: | |
| self._backends_list = self._active_api_client.list_backends() |
There was a problem hiding this comment.
I did it this way because an instance passed to backends() could be a different instance from the current active api instance.
There was a problem hiding this comment.
hmmm I might be missing something, but it looks like both lines are calling the current active client?
There was a problem hiding this comment.
I think we need to handle the case where:
service = QiskitRuntimeService() # No default instance
service.backends(instance="instance1") # backends_list will only have backends from instance1
service.backends(instance="instance2") # backends_list needs to be refetched because the instance passed in is not the active instanceIf self._backends_list is not None but has the backends from a different instance the backends from the wrong instance would be returned.
There might be a better way to handle this, as the way I did it here is not the most intuitive.
| @@ -200,7 +199,8 @@ def _discover_backends_from_instance(self, instance: str) -> List[str]: | |||
| new_client = self._create_new_cloud_api_client(instance) | |||
| self._api_clients.update({instance: new_client}) | |||
| self._active_api_client = new_client | |||
| return self._active_api_client.list_backends() | |||
| self._backends_list = self._active_api_client.list_backends() | |||
| return [backend["name"] for backend in self._backends_list] | |||
| # On staging there some invalid instances returned that 403 when retrieving backends | |||
There was a problem hiding this comment.
We still have to store the /backends API response somewhere in the service object. I think it's best to handle that here in _discover_backends_from_instance()?
There was a problem hiding this comment.
You meant better to handle that here than in _discover_backends_from_instance(), right? I agree
Summary
Messed up my other two branches. This method is now significantly faster because we don't need to fetch every single backend configuration and compare the number of pending jobs.
Details and comments
Fixes #2240