Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the CONFIG_DB_INIT flag dependency on db_migrator #2959

Merged
merged 2 commits into from
Sep 10, 2023

Conversation

vivekrnv
Copy link
Contributor

What I did

db_migrator should not depend on CONFIG_DB_INITIALIZED flag. get_hwsku api call busy waits on that flag

How I did it

Replace get_hwsku call with get_localhost_info call which takes in a custom config_db object

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@@ -91,7 +91,9 @@ def __init__(self, namespace, socket=None):
self.asic_type = version_info.get('asic_type')
if not self.asic_type:
log.log_error("ASIC type information not obtained. DB migration will not be reliable")
self.hwsku = device_info.get_hwsku()

# TODO: Update get_hwsku API to take in a custom configDB object
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this TODO left? How will custom configDB help unblock the busy wait of get_hwsku?

Copy link
Contributor Author

@vivekrnv vivekrnv Aug 30, 2023

Choose a reason for hiding this comment

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

When get_hwsku() is used, it calls get_localhost_info and it calls ConfigDBConnector.connect() which busy waits. self.configDB is already connected to DB and not hitting the CFG_INIT flag. Ref: https://github.com/sonic-net/sonic-utilities/blob/master/scripts/db_migrator.py#L74

I'll remove the TODO comment

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv vivekrnv requested a review from vaibhavhd August 30, 2023 18:06
@vaibhavhd
Copy link
Contributor

@vivekrnv please update the description with how this change was validated?

@liat-grozovik liat-grozovik merged commit 57b3b31 into sonic-net:master Sep 10, 2023
@vaibhavhd
Copy link
Contributor

MSFT ADO: 25357782

JunhongMao pushed a commit to JunhongMao/sonic-utilities that referenced this pull request Oct 4, 2023
)

- What I did
db_migrator should not depend on CONFIG_DB_INITIALIZED flag. get_hwsku api call busy waits on that flag

- How I did it
Replace get_hwsku call with get_localhost_info call which takes in a custom config_db object

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@StormLiangMS
Copy link
Contributor

@vivekrnv have you test with 202305?

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Oct 9, 2023

With this PR sonic-net/sonic-buildimage#16116, this change is a good to have but not a must

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

Successfully merging this pull request may close these issues.

5 participants