Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Show default hgp in active_account #349

Merged
merged 11 commits into from
May 19, 2022
Merged

Conversation

kt474
Copy link
Member

@kt474 kt474 commented May 6, 2022

Summary

Fixes #348

Details and comments

@coveralls
Copy link

coveralls commented May 6, 2022

Pull Request Test Coverage Report for Build 2335772639

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-3.3%) to 29.982%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_provider/ibm_provider.py 0 4 0.0%
Totals Coverage Status
Change from base Build 2334074014: -3.3%
Covered Lines: 1664
Relevant Lines: 5550

💛 - Coveralls

@@ -416,6 +416,9 @@ def active_account(self) -> Optional[Dict[str, str]]:
Returns:
A dictionary with information about the account currently in the session.
"""
if not self._account.instance:
hgp = self._get_hgp()
self._account.instance = f"{hgp._hub}/{hgp._group}/{hgp._project}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self._account.instance is never updated, even after a job is run - if a user wants to see the default hgp by calling active_account() we can use _get_hgp() which returns the default hgp

This doesn't seem like the cleanest solution but I couldn't find a better way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First hgp in the list of hgps will always be the one that is selected by default.
We need to always return this no matter what is already in self._account.
So something like below should work, please test it:

hgps = self._get_hgps()
active_account_dict = self._account.to_saved_format()
active_account_dict.update({ "instance": hgps[0].name })
return active_account_dict

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works as intended

@kt474 kt474 changed the title [WIP] Show default instance Show default hgp in active_account May 6, 2022
Copy link
Member

@rathishcholarajan rathishcholarajan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kt474 Could you please write some tests to see if the h/g/p set as instance when saving account and initialzing IBMProvider or when initializing IBMProvider with instance parameter is what is set as instance value when calling active_account?

@kt474
Copy link
Member Author

kt474 commented May 10, 2022

Test is failing here but passing locally - looking into it

hgp = self.provider._get_hgp()
IBMProvider.save_account(
token=self.dependencies.token, instance=hgp.name, overwrite=True
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would better not to save account on CI. Using something like a temporary account config file might help like here https://github.com/Qiskit/qiskit-ibm-provider/blob/main/test/unit/test_account.py#L174

token = uuid.uuid4().hex
instance = "test-hub/test-group/test-project"
with temporary_account_config_file(name=name, token=token, instance=instance):
service = FakeProvider(name=name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are testing FakeProvider whereas we should be actually testing IBMProvider.

Copy link
Member

@rathishcholarajan rathishcholarajan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rathishcholarajan rathishcholarajan merged commit d4930b3 into Qiskit:main May 19, 2022
@kt474 kt474 deleted the show-instance branch May 19, 2022 14:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way for user to view currently selected h/g/p when one is auto-selected
3 participants