fabric-testbed / InformationModel

FABRIC Information Model library
MIT License
7 stars 1 forks source link

1.4 forgiving of 1.5 or other future versions. #185

Closed ibaldin closed 1 year ago

ibaldin commented 1 year ago

Made all children of JSONField forgiving based on a set_fields(forgiving) value which by default is set to False. So normally these classes will throw exceptions if an attempt to set a non-existent field is made. However when deserializing from JSON they are more forgiving and just print warnings.

Note that because FIM normally stored unaltered GraphML, if you have a topology produced with a later version of FIM that has new fields not known to this version, it will load the graph and will print warnings every time you try to access the fields. If you want to get rid of all FIM warnings do something like this:

import logging
import fim.logging.fim_logger as fl

logger = logging.getLogger('NULLFIMLOGGER')
logger.addHandler(logging.NullHandler())

fl.set_logger(logger)

This is posted on PyPi as fabric_fim==1.4.14.

@kthare10 please test - I won't merge until you are good with these changes.

kthare10 commented 1 year ago

Tested by installing fabric-fim==1.4.14 on Release 1.4.6 container. Slice provisioning works and no errors observed about numa. I did see the following warning logs in the fablib logs even after including the logger import as suggested above.

[23:25:26] {/opt/conda/lib/python3.9/site-packages/fim/slivers/capacities_labels.py:454} WARNING - Using logger <Logger FIM (INFO)> Unable to set field numa of labels, no such field available ['bdf', 'mac', 'ipv4', 'ipv4_range', 'ipv4_subnet', 'ipv6', 'ipv6_range', 'ipv6_subnet', 'asn', 'vlan', 'vlan_range', 'inner_vlan', 'instance', 'instance_parent', 'local_name', 'local_type', 'device_name', 'bgp_key', 'account_id', 'region', 'usb_id']
ibaldin commented 1 year ago

If your code (or someone's code) does logging.basicConfig() this creates a root logger which then catches these log messages regardless. In that case I think you may need to do

root_logger = logging.getLogger() # get root logger
root_logger.removeHandler(root_logger.handlers[0]) # basicConfig creates a single StreamHandler
ibaldin commented 1 year ago

This may mess up other logging, of course

kthare10 commented 1 year ago

Since, this only goes in fablib.log file, I am inclined to not change the logger as users are recommended to look at it only in case of failures. Please share your thoughts.

ibaldin commented 1 year ago

Let me look at it some more - I don't quite understand the root logger behavior - I can see that calling logging.basicConfig() does what I described above, but I do not understand why the log messages go to the root logger.

ibaldin commented 1 year ago

Seems like this is the logging behavior - if a root logger has a handler - it intercepts all messages. logging.basicConfig() installs a root logger with a stream handler and then it intercepts messages. Sounds like what you are saying is if we mess with the root logger we will turn off logging elsewhere, which is undesirable - so we don't have any choice, I think, but to live with these log messages.

ibaldin commented 1 year ago

We may need a ticket for fablib or other libraries to make sure they do not do logging.basicConfig() and use root loggers in the future. Instead create module-specific loggers and configure them individually.

kthare10 commented 1 year ago

For now, I am pushing a pypi version of fabrictestbed-extensions with updated fim dependencies.