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.

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

Jump to bottom

[yang] extend ConfigMgmt constructor to pass YANG options #2118

Merged
merged 1 commit into from
May 25, 2022
Merged

[yang] extend ConfigMgmt constructor to pass YANG options #2118

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
7 changes: 4 additions & 3 deletions config/config_mgmt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class ConfigMgmt():
to verify config for the commands which are capable of change in config DB.
'''

def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True):
def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True, sonicYangOptions=0):
'''
Initialise the class, --read the config, --load in data tree.

Expand All @@ -53,6 +53,7 @@ def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True):
self.configdbJsonOut = None
self.source = source
self.allowTablesWithoutYang = allowTablesWithoutYang
self.sonicYangOptions = sonicYangOptions

# logging vars
self.SYSLOG_IDENTIFIER = "ConfigMgmt"
Expand All @@ -67,7 +68,7 @@ def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True):
return

def __init_sonic_yang(self):
self.sy = sonic_yang.SonicYang(YANG_DIR, debug=self.DEBUG)
self.sy = sonic_yang.SonicYang(YANG_DIR, debug=self.DEBUG, sonic_yang_options=self.sonicYangOptions)
# load yang models
self.sy.loadYangModel()
# load jIn from config DB or from config DB json file.
Expand Down Expand Up @@ -280,7 +281,7 @@ def get_module_name(yang_module_str):
"""

# Instantiate new context since parse_module_mem() loads the module into context.
sy = sonic_yang.SonicYang(YANG_DIR)
sy = sonic_yang.SonicYang(YANG_DIR, sonic_yang_options=self.sonicYangOptions)
module = sy.ctx.parse_module_mem(yang_module_str, ly.LYS_IN_YANG)
return module.name()

Expand Down
3 changes: 2 additions & 1 deletion sonic_package_manager/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
import pkgutil
import tempfile
import yang as ly
from inspect import signature
from typing import Any, Iterable, List, Callable, Dict, Optional

Expand Down Expand Up @@ -1002,7 +1003,7 @@ def get_manager() -> 'PackageManager':
docker_api = DockerApi(docker.from_env(), ProgressManager())
registry_resolver = RegistryResolver()
metadata_resolver = MetadataResolver(docker_api, registry_resolver)
cfg_mgmt = config_mgmt.ConfigMgmt(source=INIT_CFG_JSON)
Copy link

Choose a reason for hiding this comment

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

I did not understand this use.
So ConfigMgmt searches for yang models at a fixed location 'YANG_DIR = "/usr/local/yang-models"', then in what case it is an error ?.

Kindly elaborate, thx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly
ConfigMgmt searches for yang models in current working directory AND "/usr/local/yang-models"

Copy link
Contributor

@ghooo ghooo May 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Since the expectation is to only read schema from /user/local/yang-models let's disable reading schema from CWD (current working dir) in sonic-yang-mgmt directly, no need to create a flags object and pass it.

libyang ref: https://netopeer.liberouter.org/doc/libyang/master/html/group__contextoptions.html#ga08ef1a18adb7dac8e9f4a2c9522875ba
PR to add SonicYangOptions needs to be reverted: sonic-net/sonic-buildimage#10359

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ghooo I have considered this option earlier, but after the discussion I chose the current approach. It looks more flexible to me and allows to pass different yang options to sonic-yang-mgmt

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this change will benefit all users of sonic_yang library. It would be better if we move the change to the library directly. Approving to get this unblocked.

cfg_mgmt = config_mgmt.ConfigMgmt(source=INIT_CFG_JSON, sonicYangOptions=ly.LY_CTX_DISABLE_SEARCHDIR_CWD)
cli_generator = CliGenerator(log)
feature_registry = FeatureRegistry(SonicDB)
service_creator = ServiceCreator(feature_registry,
Expand Down