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

Log manager for Python based nodes #631

Merged
merged 8 commits into from
Jan 25, 2020
Merged

Log manager for Python based nodes #631

merged 8 commits into from
Jan 25, 2020

Conversation

ChemicalXandco
Copy link
Contributor

@ChemicalXandco ChemicalXandco commented Sep 12, 2019

Description

There is currently no easy way for nodes that are executed in python to use the log file, this system makes adding a message as simple as chunk.logger.info('message'), currently the Publish node uses print() but this is not very helpful because it is not stored in a file that can be viewed afterwards.

chunk.logManager.waitUntilCleared()
chunk.logger.setLevel(chunk.logManager.textToLevel(chunk.node.verboseLevel.value))

chunk.logger.info('info message')
chunk.logger.warning('warning message')
chunk.logger.error('error message')

chunk.logManager.makeProgressBar(100)
for i in range(101):
    chunk.logManager.updateProgressBar(i)
chunk.logManager.completeProgressBar()

becomes

[18:06:47.016][info] info message
[18:06:47.022][warning] warning message
[18:06:47.028][error] error message
0%   10   20   30   40   50   60   70   80   90   100%
|----|----|----|----|----|----|----|----|----|----|
***************************************************

Features list

  • LogManager class to manage the logs
  • Simulation of boost::progress_display within LogManager
  • Updated Publish node to use this system

Implementation remarks

  • Progress bar can be used while adding messages
  • Log clears automatically every time the chunk is ran

I do welcome feedback because I am not sure if this is the best way to implement this.

ChemicalXandco and others added 4 commits September 11, 2019 16:41
also fix error not returning proper information (chunk.node.inputFiles.value - > [i.value for i in chunk.node.inputFiles.value])
@fabiencastan
Copy link
Member

Hi @ChemicalXandco ,
That's an interesting feature that will be useful to create more python-only nodes.

In term of implementation, it would be nice if we could rely on a standard logging object.
With logging, you can directly write to file, like
logging.basicConfig(filename='/path/to/node/cache/log', level=logging.INFO).
And create a logging object in your class with something like log = logging.getLogger(uniqueNodeId).

It would be good also to expose a verbose level parameter on the node, as on the C++ nodes.
Best,

@@ -142,6 +237,7 @@ def __init__(self, node, range, parent=None):
super(NodeChunk, self).__init__(parent)
self.node = node
self.range = range
self.log = LogManager(self)
Copy link
Member

Choose a reason for hiding this comment

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

    def __init__(self, node, range, parent=None):
        ...
        self.logManager = LogManager(self)
        ...

    @property
    def logging(self):
        return self.logManager.logger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have called the property logger instead of logging because it makes more sense imo.

Copy link
Member

Choose a reason for hiding this comment

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

yes, I agree.

shutil.copyfile(iFile, oFile)
print('Publish end')
chunk.log.add('Publish end')
Copy link
Member

Choose a reason for hiding this comment

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

chunk.logging.info('Publish end')

also allow progress bar to be used even while messages are added
@TigerVersusT
Copy link

@ChemicalXandco a stupid question, it seems that your logManager class is used for python based nodes only, how about CLI based node? Is it suitable to use your class for log implementtation in my own CLI nodes? And could you tell me somthing about how do nodes like "Texturing" in meshroom handle the log message.Thanks.

@ChemicalXandco
Copy link
Contributor Author

@TigerVersusT The C++ side of the software (AliceVision framework) uses it's own logging system. By 'CLI based node', I'm assuming you mean an executable file not from the AliceVision framework. In that case, the best approach imo would be to forward the messages from stdout of the process to the logger implemented here.

@TigerVersusT
Copy link

@ChemicalXandco Thanks for your help!

@ChemicalXandco ChemicalXandco deleted the python_logging branch June 20, 2020 11:44
@fabiencastan fabiencastan added this to the Meshroom 2020.1.0 milestone Aug 12, 2020
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.

3 participants