tcalmant / ipopo

iPOPO: a Service-Oriented Component Model for Python
https://ipopo.readthedocs.io/
Apache License 2.0
69 stars 28 forks source link

iPOPOv2 development #109

Closed acutaia closed 3 weeks ago

acutaia commented 4 years ago

This issue will be used to synchronize with the process of developing iPOPOv2. I've decided to start working with those bundles:

after that I think we should move to the bundles of the pelix shell What do you think? @tcalmant @scottslewis

scottslewis commented 4 years ago

Since I can't commit to follow this development closely, but would like to have a predictable outcome, can we establish clearly what's going to happen here wrt releases and content? i.e. 1 or more releases, what versions (1 and 2?, 2 only?)...requiring which versions of python 2 and 3?), when these are likely to happen, and (eventually) who is going to commit to doing what/when?

tcalmant commented 4 years ago

Hi,

acutaia commented 4 years ago

Hi,

Dear @scottslewis , like Thomas said the 2.0.0 release won't have an ETA cause convert already existing synchronous code to async it's not fast and easy to achieve cause this process will be bug-prone at the beginning and every design choice will have to be well thought. So i prefer spend more time and be sure that the result is good than do everything without a good plan in mind.

Dear @tcalmant , I'm totally agree with yours idea cause i think we should do everything step by step, in fact I've taken sometime before open this issue cause i wanted to have a list of tasks in mind.

tcalmant commented 4 years ago

Hi,

I was thinking to introduce this to handle the pelix.shell I/O , what do you think?

The shell I/O is handled by the IOHandler class, defined in pelix.shell.beans, which supports file-like objects. In fact, it just calls readline, write and flush. For example, the XMPP shell defines the _XmppOutStream and _XmppInStream classes to provide these methods. The IOHandler class can be updated to provide use asyncio, but using aioconsole can work to implement a specific IOHandler for pelix.console, not for the whole shell providers.

I like the idea of aiohttp also we could use async sockets and opening connection

I think the async sockets will have to be used in the Remote Shell.

scottslewis commented 4 years ago
    As a side note, I don't think we'll have an `async` implementation of the remote services in 2.0.0, we'll just ensure the current version can call a coroutine (an asynchronous method) like a synchronous one. To implement _async_ remote services, we'll need an asynchronous HTTP service, which would come later.

Not sure whether you knew this, but OSGi Remote Services spec v7:

https://osgi.org/specification/osgi.cmpn/7.0.0/service.remoteservices.html

Has a section on asynchronous remote services:

https://osgi.org/specification/osgi.cmpn/7.0.0/service.remoteservices.html#d0e1407

Quick summary: A remote service exporter can export a remote service with 'osgi.async' standard service property, and then the client/consumer of this service can use (in java) return types like:

org.osgi.util.promise.Promise

java.util.concurrent.Future

java.util.concurrent.CompletionStage

java.util.concurrent.CompletableFuture

In the RSA impl that I added to iPopo, I added support in the RSA impl for Python class as a possible return type with Python on client:

concurrent.futures.Future

The effect being that for RSA distribution providers (currently py4j and xmlrpc) if a service is exported with 'osgi.async' intent (as per OSGi spec) then the proxy that gets created on import will return an instance of type concurrent.futures.Future.

There's an example of usage of such a proxy injected as self._helloservice here:

https://github.com/tcalmant/ipopo/blob/v1/samples/rsa/helloconsumer_xmlrpc.py#L54

Short story: RSA already has support for OSGi Remote Services R7 Async Remote Services. Adding other distribution providers that support the same approach is a relatively simple matter.

acutaia commented 4 years ago

Hi,

The IOHandler class can be updated to provide use asyncio, but using aioconsole can work to implement a specific IOHandler for pelix.console, not for the whole shell providers.

Yes, i had a a freudian slip sorry. However I've done some commits to drop the support of python <3.7, but the building upon travis-ci with python 3.8 always fails when it tries to do the discovery fails:

To @scottslewis I heard that the OSGi had an asynchronous remote service, the thing is that Java uses threads more efficiently than python. While I've been working on dropping the support from Python <3.7 I read carefully your code. asyncio supports its own version of concurrent.futures.Future which will enhance the efficiency of your code so I think that we should implement it in the future. By now I think we should do it step by step till the release of version 2.0.0 by developing till the point @tcalmant pointed out .

Adding other distribution providers that support the same approach is a relatively simple matter.

I totally agree with you

scottslewis commented 4 years ago

To @scottslewis I heard that the OSGi had an asynchronous remote service, the thing is that Java uses threads more efficiently than python. While I've been working on dropping the support from Python <3.7 I read carefully your code. asyncio supports its own version of concurrent.futures.Future which will enhance the efficiency of your code so I think that we should implement it in the future. By now I think we should do it step by step till the release of version 2.0.0 by developing till the point @tcalmant pointed out .

@Angeloxx92 FWIW I agree with you...since what you are proposing doing with asyncio is essentially to improve performance. There's no reason that I can think of that performance improvement wrt distributino provider usage of concurrent.futures.Future would be a problem...as long as Future semantics are maintained of course.

Adding other distribution providers that support the same approach is a relatively simple matter.

I totally agree with you

Sounds good. It might be useful to create an asyncio-based xmlrpc distribution provider (which is currently based on pelix.http.basic). Actually, come to think about it, if a new version of pelix.http.basic based upon asyncio was available it would require no changes at all in the xmlrpc provider to benefit.

tcalmant commented 4 years ago

@Angeloxx92 :

building upon travis-ci with python 3.8 always fails when it tries to do the discovery

Yes, I saw that. I propose we remove the 3.8 tests for now, to focus on the port to 3.7. We'll handle this issue a bit later as it might come either from the underlying libraries we use (which we should upgrade as necessary) or from Travis itself.

acutaia commented 4 years ago

Hi,

Yes, I saw that. I propose we remove the 3.8 tests for now, to focus on the port to 3.7. We'll handle this issue a bit later as it might come either from the underlying libraries we use (which we should upgrade as necessary) or from Travis itself.

to @tcalmant I agree with you. However , I'm almost done working on framework.py (i think 3 days of work) , the synchronous functions are wrapping the asynchronous one and from outside they'll work synchronously, while under the hood they'll still be asynchronously. If you have some question tell me Example

  def start(self):
        """
        Starts the bundle. Does nothing if the bundle is already starting or
        active.

        :raise BundleException: The framework is not yet started or the bundle
                                activator failed.
        """
        if self.__loop.is_running():
            # I'm in another thread, so I'm scheduling the coroutine
            future = asyncio.run_coroutine_threadsafe(self.async_start(), self.__loop)
            try:
                # Waiting for the end of the coroutine 
                result = future.result()
            except BundleException:
                raise BundleException
        else:
            try:
                self.__loop.run_until_complite(self.async_start())
            except BundleException:
                raise BundleException

async def async_start(self):
        """
        Async Starts the bundle. Does nothing if the bundle is already starting or
        active.

        :raise BundleException: The framework is not yet started or the bundle
                                activator failed.
        """
        if self.__framework._state not in (Bundle.STARTING, Bundle.ACTIVE):
            # Framework is not running
            raise BundleException(
                "Framework must be started before its bundles"
            )

        async with self._lock:
            if self._state in (Bundle.ACTIVE, Bundle.STARTING):
                # Already started bundle, do nothing
                return

            # Store the bundle current state
            previous_state = self._state

            # Starting...
            self._state = Bundle.STARTING
            starting = self.__loop.create_task(self._fire_bundle_event(BundleEvent.STARTING))
            await starting

            # Call the activator, if any
            activator = self.__loop.create_task(self.__get_activator_method("start"))
            starter = await activator
            if starter is not None:
                try:
                    # Call the start method
                    starter(self.__context)
                except (FrameworkException, BundleException):
                    # Restore previous state
                    self._state = previous_state

                    # Re-raise directly Pelix exceptions
                    _logger.exception(
                        "Pelix error raised by %s while starting", self.__name
                    )
                    raise
                except Exception as ex:
                    # Restore previous state
                    self._state = previous_state

                    # Raise the error
                    _logger.exception(
                        "Error raised by %s while starting", self.__name
                    )
                    raise BundleException(ex)

            # Bundle is now active
            self._state = Bundle.ACTIVE
            started = self.__loop.create_task(self._fire_bundle_event(BundleEvent.STARTED))
            await started

to @scottslewis

Sounds good. It might be useful to create an asyncio-based xmlrpc distribution provider (which is currently based on pelix.http.basic). Actually, come to think about it, if a new version of pelix.http.basic based upon asyncio was available it would require no changes at all in the xmlrpc provider to benefit.

You're right , but maybe we should also add async methods to the xmlrpc provider. However my goal is to not break application built upon iPOPOv1 but to increase their performances and give them the chance of use totally async methods and not only synchronous one

scottslewis commented 4 years ago

You're right , but maybe we should also add async methods to the xmlrpc provider.

Hi @Angeloxx92 . The xmlrpc provider simply turns around and makes http requests after serializing method args (and deserialization, etc on the server side).

The current xmlrpc provider (and lots of other things) would benefit from using asyncio to implement the ipopo http service...which is what xmlrpc uses to make the http request/response calls.

I don't think there would be any significant benefit to using asyncio on the serialization/deserialization of the args and return values of the xmlrpc provider...since typically this happens in memory.

acutaia commented 4 years ago

Dear @tcalmant @scottslewis, I've just finish to work on framework.py , now I'm in the debugging phase , when I'll finish it I'll do the commit and I'll work on pelix.internals in order to fully convert framework.py . I'll give you updates during the next days. Have a nice day

tcalmant commented 4 years ago

Thanks for the news!

acutaia commented 4 years ago

Dear @tcalmant , It's been days that I'm trying to make all the tests for the framework run without errors, but there is no way to make it wrapping asynchronous code with synchronous calling loop.run_until_complete(coroutine()) Example:

def start(self) -> None:
        """
        Starts the bundle. Does nothing if the bundle is already starting or
        active.
        :raise BundleException: The framework is not yet started or the bundle
                                activator failed.
        """
        if threading.current_thread() is threading.main_thread():
            # I'm in the main thread
            return loop.run_until_complete(self.async_start())

        else:
            # I'm in another thread, so I'm scheduling the coroutine
            if loop.is_running():
                start: Future = asyncio.run_coroutine_threadsafe(self.async_start(), loop)
                # Waiting for the end of the coroutine
               return start.result()
           else:
               return loop.run_until_complete(self.async_start())

Cause, if I'm on another thread(just to make it simple, call it thread2) and the loop in the main_thread is not running because it has finished to execute loop.run_until_complete(coroutine()), the thread2 will try to start the loop to run the coroutine that was trying to schedule, but if in the meanwhile the main thread starts the loop, an exception raises cause it's like the loop is already running. Test failed

To solve this problems there are 2 options:

  1. put in the main thread the asyncio's loop , use loop.run_forever() and schedule from other threads the coroutines asyncio.run_threadsafe(coroutine(), loop)
  2. write the code in a totally asynchronous way losing part of the compatibility with the already existing code written by the users In both cases the tests will have to be rewritten. Personally i prefer option 2 cause it's easier to accomplish and faster. If you need further explanations, tell me.
tcalmant commented 4 years ago

Hi @Angeloxx92 ,

Losing the compatibility is a big issue, but I'm OK to prototype it, to see where we can go in a full-async version (both in terms of usage possibilities and performances). We'll then see if we can prepare a compatibility layer above the full-async version.

acutaia commented 4 years ago

Dear @tcalmant , I'll start to rewrite the tests of the framework and I'll commit the first update of it as soon as possible.

We'll then see if we can prepare a compatibility layer above the full-async version.

Do you have any idea about it?What should do the compatibility layer? I'm asking this cause I'd like to have all clear in my mind.

tcalmant commented 3 weeks ago

Closing issue as development of v2 is stalled and considered abandoned. In the case of future developments related to async should now be based on v3 branch and for Python 3.10+