OpenAADL / ocarina

AADL model processor: mappings to code (C, Ada); Petri Nets; scheduling tools (MAST, Cheddar); WCET; REAL
http://www.openaadl.org
Other
66 stars 29 forks source link

Python API: general improvements #30

Closed yoogx closed 8 years ago

yoogx commented 9 years ago

This issue is to track request for improvements: which function should return something?

others?

maxime-esa commented 9 years ago

watch out, Makefile.am in resources/runtime/python/ has a typo line 6 ( + = instead of +=)

JH: corrected, thanks

maxime-esa commented 9 years ago

TODO: in ocarina.py, add some warning to the user if a module cannot be imported, instead of catching ImportError and discarding it

maxime-esa commented 9 years ago

Issue in function analyze: the return values are not understood (and wrong):

this:

def analyze ():
    '''Analyze models'''
    info = io.BytesIO()
    error = io.BytesIO()
    raisedError = []
    res = ''
    with std_redirector(info,error):
        try:
            res = libocarina_python.analyze ()
        except:
            raisedError.append(getErrorMessage()) 
    stderrMsg = sortStderrMessages(error.getvalue().decode('utf-8'))
    if stderrMsg[1]!=[]:
        raisedError.append(stderrMsg[1])
    return [ res , info.getvalue().decode('utf-8'), stderrMsg[0] , 
        raisedError ]

returns this:

res= True
info= Model analyzed sucessfully

err= []
raised= [[u'InterfaceView.aadl:56:07: Warning: Source_Language is not a list while the corresponding property name at programming_properties.aadl:61:02 is a list.\n\nInterfaceView.aadl:56:07: Warning: The value of Source_Language has been converted into a list.\n\nInterfaceView.aadl:87:07: Warning: Source_Language is not a list while the corresponding property name at programming_properties.aadl:61:02 is a list.\n\nInterfaceView.aadl:87:07: Warning: The value of Source_Language has been converted into a list.\n\nInterfaceView.aadl:112:07: Warning: Source_Language is not a list while the corresponding property name at programming_properties.aadl:61:02 is a list.\n\nInterfaceView.aadl:112:07: Warning: The value of Source_Language has been converted into a list.']]

The first value (res) is OK - General output status The second (info) is a string - corresponding to the general status - OK, why not but then err and raised (called raisedError in ocarina.py) are not clear Why is the last argument of type [['long string with all warnings']] ?

Thanks

maxime-esa commented 9 years ago

Not sure what you are doing with stderr in the std_redirector context manager, but this messes up the logging functionality from stdlib and it becomes impossible to use it after calling an ocarina function:

Traceback (most recent call last):
  File "/usr/lib/python2.7/logging/__init__.py", line 880, in emit
    stream.write(fs % msg)
ValueError: I/O operation on closed file

This is probably due to the sys.stderr.close() that you do (in ocarina_common_tools.py), but I can't tell for sure. Perhaps related to this discussion: http://bugs.python.org/issue6333

maxime-esa commented 9 years ago

The return value of the instantiate function seems wrong:

success, _, _, _ = ocarina.instantiate("interfaceview.others")

The first return value (called success here) was expected to be a boolean, but it is a empty string.

Ellidiss commented 9 years ago

Regarding the output of the API functions, here is a part of the to be published design report:

All the functions available in the API have the same template for their return value. It is a list of the form { [result, empty string if none], [stdout from Ocarina], [warnings from ocarina], [errors from Ocarina]} which allows an homogeneous management of error.

In stderr, everything that is not considered as warning is considered an error. In your exemple, the warnings are prefixed with the aadl file name. In the API, to be considered as warning, a line in the stderr shall start with "warning:". Has the warning output been modified or do I missed something?

Regarding the context manager, in order to get stderr and stdout, both channels are redirected to two new python streams. Seems that I forget to replug stdout/err to the old channels after the function is called. Will look at it.

Regarding the type of the result, it has not been defined to be a boolean. I just forward the result gived by Ocarina as a string. If the awaited result is false when error occurs and true in other cases, just let me know.

Ellidiss commented 9 years ago

Regarding the logging functionnality of the libc, in the std_redirector, the stdout/err are repluged to the original channels at the end of the ocarina function call. Does the logging facility open a channel on the stderr prior to call the function in Ocarina and uses it after the call return? If that is the case, yes, the call to sys.stderr.close() may break the channel from the logging facility.

maxime-esa commented 9 years ago

Regarding the type of the result, it has not been defined to be a boolean. I just forward the result gived by Ocarina as a string. If the awaited result is false when error occurs and true in other cases, just let me know.

In the example shown above, "analyse" seems to return a boolean, not a string (res = True), while the 2nd return value is the string returned by ocarina (Model analyzed successfully). I think it is better like this - what other string could be emitted in the first return value?

maxime-esa commented 9 years ago

It is a list of the form { [result, empty string if none], [stdout from Ocarina], [warnings from ocarina], [errors from Ocarina]}

But this is not what is actually sent.

1) Typewise, what you send is:

[str (or bool), str, list, list]

and NOT

set(list, list, list, list)    # {} is for set , [] is for list in your above interface  

2) Check the example above, semantically speaking it is also not consistent: you say the last parameter is the list of errors from ocarina, but in fact it is the warnings (and the warnings list is empty)

3) You send lists (for warnings and errors) but in fact you have only one element which is a single string. You should split the string into a tuple/list- using "warning:" to split.

Ellidiss commented 9 years ago

Maybe I used a wrong style, I really wanted to say the result is a list of four elements which are:

Regarding the warnings, I mentionned the problem of the signature of the warning message four comments above. In the current code, I consider that a warning line starts with "warning:" whereas in your exemple it seems to start by an AADL file name followed by a line and a column number and only after that by "warning:". Then I consider all the stderr lines to be error messages. That explains the content of the result (no warnings in the warning "list" but a lot of warnings in the error "list"). For the type of the two last elements of the result, I agree that it is a bug. Now, what I need to know is the signature of a warning message written by Ocarina in stderr (Jérôme?).

JH: what do you mean by signature ? all warning messages should be prefixed by "warning", if not open a ticket

Ellidiss commented 9 years ago

Regarding the first item of the result list of the API function. Its underling type depends on the called Ocarina function. For instances:

As such, this item cannot be considered neither as a boolean nor as a string. Its type is context dependant. Except if there is a specific requirement, I do not know what should be changed.

maxime-esa commented 9 years ago

(Topic: First returned item)

OK to be context-dependent. In the case of instantiate I thought that for consistency it should return a boolean but if the inconsistency is in Ocarina and not in your part, I guess there is nothing you can do (But Jerome: I would suggest to return a boolean for this function, just like analyze)

JH: not a problem to return a boolean, you asked it also for analyze in the past (see the history). This is now implemented, but not tested. There might be trivial bug to solve

maxime-esa commented 9 years ago

Regarding the logging functionnality of the libc, in the std_redirector, the stdout/err are repluged to the original channels at the end of the ocarina function call. Does the logging facility open a channel on the stderr prior to call the function in Ocarina and uses it after the call return? If that is the case, yes, the call to sys.stderr.close() may break the channel from the logging facility.

I am not sure how the logging function is implemented, but I think it is something like that. When you use it, you initialize it at the begining of the application:

log = logging.getLogger(__name__)
console = logging.StreamHandler(sys.__stdout__)
console.setFormatter(ColorFormatter())
log.addHandler(console)

And there anywhere in your application you can do:

log.info('hello')
log.warning('world')
log.error ('....')

So I guess that if you close stdout and stderr in the middle of the application, the logging cannot recover the channel...

maxime-esa commented 9 years ago

Logging: I found a workaround, by using a StringIO in the logger, and print it at the end of the application:

Initialization of the logger:

    log = logging.getLogger(__name__)
    stream = StringIO.StringIO()
    console = logging.StreamHandler(stream)
    console.setFormatter(ColorFormatter())
    log.addHandler(console)

and before the application exits:

    print stream.getvalue()
    stream.flush()
    stream.close()
Ellidiss commented 9 years ago

(Topic: signature of error/warning)

I found two signatures for warning messages:

I change my code to take the two versions. But if you confirm that it is a bug, I will create a ticket.

JH: It is not, in some occasion we do not have access to the line number. But I guess there is still a minor bug: for the second occurrence, there should also be "warning" printed. Can you open a ticket with an example ? Thanks

maxime-esa commented 9 years ago

@yoogx : the "Warning" is printed, as seen in example above: InterfaceView.aadl:56:07: Warning: Source_Language is not a list while the....

yoogx commented 8 years ago

Closing this old ticket