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

[Junos] Make _load_candidate() support loading of XML files with headers that have encoding information #1672

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+11 −6
Open

[Junos] Make _load_candidate() support loading of XML files with headers that have encoding information #1672

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
796350f
[Junos] Make _load_candidate() support loading of XML files with head…
hkam40k Jun 15, 2022
779a6ee
Merge branch 'develop' into junos-fix-xml-load
mirceaulinic Jul 6, 2022
0a5b4a9
Merge branch 'develop' into junos-fix-xml-load
mirceaulinic Mar 21, 2024
4734f1a
Merge branch 'develop' into junos-fix-xml-load
ktbyers Mar 26, 2024
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
17 changes: 11 additions & 6 deletions napalm/junos/junos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,21 @@ def _detect_config_format(self, config):
return fmt

def _load_candidate(self, filename, config, overwrite):
fmt = None
if filename is None:
configuration = config
else:
with open(filename) as f:
configuration = f.read()
try:
configuration = etree.parse(f).getroot()
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's best to always try to load any file. The format should be still detected correctly by the _detect_config_format helper, so why not just update the code below (line 260) to

            fmt = self._detect_config_format(configuration)

            if fmt == "xml":
-               configuration = etree.XML(configuration)
+               configuration = etree.parse(f).getroot()

Copy link
Author

Choose a reason for hiding this comment

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

For this to work the f file handle needs to be kept around, and the way to load the XML is a bit different when loading from file or loading from string (etree.parse(fh).getroot() or etree.XML(conf_string)), so I don't see a clean way to handle all cases in a simple way.

It could be modified though to first read the file, detect format, and parse XML from file handle only when approriate. In that case I would do something like this:

    def _load_candidate(self, filename, config, overwrite):
        if filename is None:
            configuration = config
            fmt = self._detect_config_format(configuration)
            if fmt == "xml":
                # Note: configuration cannot have encoding information, since it is already in string-format.
                configuration = etree.XML(configuration)
        else:
            with open(filename) as f:
                configuration = f.read()
                fmt = self._detect_config_format(configuration)
                if fmt == "xml":
                    # XML header encoding is evaluated, if present.
                    configuration = etree.parse(f).getroot()

        if (
            not self.lock_disable
            and not self.session_config_lock
            and not self.config_private
        ):
            # if not locked during connection time, will try to lock
            self._lock()

        try:
            if self.config_private:
                try:
                    self.device.rpc.open_configuration(private=True, normalize=True)
                except RpcError as err:
                    if str(err) == "uncommitted changes will be discarded on exit":
                        pass

            self.device.cu.load(
                configuration,
                format=fmt,
                overwrite=overwrite,
                ignore_warning=self.ignore_warning,
            )
        except ConfigLoadError as e:
            if self.config_replace:
                raise ReplaceConfigException(e.errs)
            else:
                raise MergeConfigException(e.errs)

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's try this way.

fmt = "xml"
except etree.XMLSyntaxError:
configuration = f.read()

if not fmt:
fmt = self._detect_config_format(configuration)
if fmt == "xml":
configuration = etree.XML(configuration)

if (
not self.lock_disable
Expand All @@ -248,11 +258,6 @@ def _load_candidate(self, filename, config, overwrite):
self._lock()

try:
fmt = self._detect_config_format(configuration)

if fmt == "xml":
configuration = etree.XML(configuration)

if self.config_private:
try:
self.device.rpc.open_configuration(private=True, normalize=True)
Expand Down