Skip to content

[db_migrator] Add version_202411_02 #3864

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.

Sign up for GitHub

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

Merged
qiluo-msft merged 1 commit into from
Jun 9, 2025
Merged

[db_migrator] Add version_202411_02 #3864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions scripts/db_migrator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1293,6 +1293,14 @@ def version_202411_01(self):
Version 202411_01.
"""
log.log_info('Handling version_202411_01')
self.set_version('version_202411_02')
Copy link
Contributor

@vaibhavhd vaibhavhd Jun 2, 2025

Choose a reason for hiding this comment

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

I am confused. Why does 202411 branch have a different code for the DB version 202411_01.

I think the version view for a specific DB versions should be same across the branches. Otherwise it not just gets confusing to follow, but could also mean two different things, on same version. When new branches are cut they will have different config migration for the same version. That's not expected?

sonic-utilities/scripts/db_migrator.py

Line 1277 in e6fb8f9

def version_202411_01(self):

Copy link
Contributor Author

@benle7 benle7 Jun 3, 2025

Choose a reason for hiding this comment

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

Hi @vaibhavhd ,

I'm not sure I fully understand what you mean.

I didn't change any existing migration logic for version_202411_01.
I only added version_202411_02() to the migration flow so that if config_db.json starts at version_202411_02,
the migrate() function (which uses getattr(self, version)()) can proceed without error.

Could you help clarify your concern and what you'd suggest as the preferred approach in this case?

Thanks!

Copy link
Contributor

@vaibhavhd vaibhavhd Jun 9, 2025

Choose a reason for hiding this comment

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

I am assuming that my question is best answered by @stepanblyschak.
I had it when I check your PR and see difference in version definitions. I think we are missing an enforcement or guideline around this kind of change.

I will sign off on your change as it is not responsible for the confusion that I have.

return 'version_202411_02'

def version_202411_02(self):
"""
Version 202411_02.
"""
log.log_info('Handling version_202411_02')
self.set_version('version_202505_01')
return 'version_202505_01'

Expand Down
16 changes: 8 additions & 8 deletions tests/db_migrator_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ def setup_class(cls):
{'v1': 'version_202311_02', 'v2': 'version_202311_01', 'result': True},
{'v1': 'version_202305_01', 'v2': 'version_202311_01', 'result': False},
{'v1': 'version_202311_01', 'v2': 'version_202305_01', 'result': True},
{'v1': 'version_202405_01', 'v2': 'version_202411_01', 'result': False},
{'v1': 'version_202411_01', 'v2': 'version_202405_01', 'result': True},
{'v1': 'version_202411_01', 'v2': 'version_master_01', 'result': False},
{'v1': 'version_202405_01', 'v2': 'version_202411_02', 'result': False},
{'v1': 'version_202411_02', 'v2': 'version_202405_01', 'result': True},
{'v1': 'version_202411_02', 'v2': 'version_master_01', 'result': False},
{'v1': 'version_202311_01', 'v2': 'version_master_01', 'result': False},
{'v1': 'version_master_01', 'v2': 'version_202311_01', 'result': True},
{'v1': 'version_master_01', 'v2': 'version_master_02', 'result': False},
Expand Down Expand Up @@ -386,7 +386,7 @@ def test_dns_nameserver_migrator(self):
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'dns-nameserver-expected')
expected_db = Db()
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202411_01')
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202411_02')
resulting_keys = dbmgtr.configDB.keys(dbmgtr.configDB.CONFIG_DB, 'DNS_NAMESERVER*')
expected_keys = expected_db.cfgdb.keys(expected_db.cfgdb.CONFIG_DB, 'DNS_NAMESERVER*')

Expand Down Expand Up @@ -905,15 +905,15 @@ def test_init(self, mock_args):
@mock.patch('swsscommon.swsscommon.SonicDBConfig.isInit', mock.MagicMock(return_value=False))
@mock.patch('swsscommon.swsscommon.SonicDBConfig.initialize', mock.MagicMock())
def test_init_no_namespace(self, mock_args):
mock_args.return_value = argparse.Namespace(namespace=None, operation='version_202411_01', socket=None)
mock_args.return_value = argparse.Namespace(namespace=None, operation='version_202411_02', socket=None)
import db_migrator
db_migrator.main()

@mock.patch('argparse.ArgumentParser.parse_args')
@mock.patch('swsscommon.swsscommon.SonicDBConfig.isGlobalInit', mock.MagicMock(return_value=False))
@mock.patch('swsscommon.swsscommon.SonicDBConfig.initializeGlobalConfig', mock.MagicMock())
def test_init_namespace(self, mock_args):
mock_args.return_value = argparse.Namespace(namespace="asic0", operation='version_202411_01', socket=None)
mock_args.return_value = argparse.Namespace(namespace="asic0", operation='version_202411_02', socket=None)
import db_migrator
db_migrator.main()

Expand Down Expand Up @@ -950,7 +950,7 @@ def test_dns_nameserver_migrator_minigraph(self):
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'gnmi-minigraph-expected')
expected_db = Db()
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202411_01')
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202411_02')
resulting_table = dbmgtr.configDB.get_table("GNMI")
expected_table = expected_db.cfgdb.get_table("GNMI")

Expand All @@ -966,7 +966,7 @@ def test_dns_nameserver_migrator_configdb(self):
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'gnmi-configdb-expected')
expected_db = Db()
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202411_01')
advance_version_for_expected_database(dbmgtr.configDB, expected_db.cfgdb, 'version_202411_02')
resulting_table = dbmgtr.configDB.get_table("GNMI")
expected_table = expected_db.cfgdb.get_table("GNMI")

Expand Down
Loading