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

Custom ii methods have no knowledge of the incoming connection for which they're running #30

Open
Ninjef opened this issue Nov 12, 2019 · 2 comments

Comments

@Ninjef
Copy link
Contributor

Ninjef commented Nov 12, 2019

@factory.build_ii_close and @factory.process_data do not provide the user with current_interface, but instead provide plugin.

current_interface is the tool input connection (not anchor, because an anchor can have many connections) which is currently being processed by the engine. This gets passed to ii_close in the Python SDK. It contains the incoming data, metadata, etc... for that connection.

plugin houses all of the interfaces, including current_interface. However, none of the interfaces housed by plugin are labeled as the current interface, so it is not clear which interface is the one currently being processed by the engine.

Currently, snakeplane is swallowing current_interface (which is supplied by the engine) by instead passing current_interface.parent (plugin) into the user's custom ii_close or ii_push_record method.

For batch mode, everything works fine with multiple interfaces because ii_close does not run until all incoming interfaces have completed. But for streaming mode and a potential chunk mode, the user will have to employ some tricks to ensure they process data on the correct incoming interface each time it runs.

There are several fixes. I'll take wrap_ii_close from plugin_factory as an example:

I am partial to solution #2 so check for any bias I might be injecting here!

        @_monitor("ii_close")
        @wraps(func)
        def wrap_ii_close(current_interface: object):
            try:
                current_plugin = current_interface.parent

                current_plugin.assert_all_inputs_connected()

                if current_plugin.update_only_mode:
                    return

                current_interface.completed = True

                return func(current_plugin)
            except Exception as e:
                logger.exception(e)
                raise e

        setattr(self._plugin.plugin_interface, "ii_close", wrap_ii_close)

Solution 1:

Before running return func(current_plugin), we could attach a flag to current_interface to indicate that it is the current active interface. We then would apply a flag to all other interfaces to indicate that they are not the current active interface.

Pro:

  • Does not break current snakeplane tools

Con:

  • Requires user to find the current interface

Potential Con:

  • Diverges from the way the Python SDK works

Solution 2:

Just like the engine does it, we pass current_interface into func instead of current_plugin.
ie:

    return func(current_interface)

Pros:

  • Show reduce code footprint
  • Works like the engine works

Con:

  • Will likely break many current snakeplane tools
@johnkabler
Copy link
Contributor

Per solution 1:

Why don't we simply add a property to the plugin object called "current_interface_name" and have the code attach the current_interface.name property to it?

@Ninjef
Copy link
Contributor Author

Ninjef commented Nov 14, 2019

That could work too. But per our conversation earlier I think we both agree it may not be ideal.

Here's another thought to consider. We could non-invasively add a parameter to build_ii_close which allows the developer to choose what kind of wrapper they want around their custom ii_close method:

def build_ii_close(self, func: object, wrap_type: str = "legacy"):

It would default to the current type of wrapper ("legacy" or something), which is:

            def wrap_ii_close(current_interface: object):
                try:
                    current_plugin = current_interface.parent

                    current_plugin.assert_all_inputs_connected()

                    if current_plugin.update_only_mode:
                        return

                    current_interface.completed = True

                    return func(current_plugin)
                except Exception as e:
                    logger.exception(e)
                    raise e

The most important piece, for this issue, is the line return func(current_plugin)

Another wrapper mode could be added with only one difference, return func(current_interface).

PROS:

  • Non-invasive
  • More flexibility for user

CONS:

  • New feature to support

If we go with something like this, we could still migrate to Solution 1 in my original post above, getting rid of this toggle entirely (ie version 1.x of snakeplane has the toggle, and version 2.x of snakeplane has no toggle and always passes in current_interface).

Exposing the toggle in 1.x would allow users to use it without breaking their current tools, and give us time to evaluate it's usefulness before deciding what to do for the next major release.

Any thoughts?

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

No branches or pull requests

2 participants