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

AcquisitionBase - missing 'step_name' in constructor __init__ #8

Open
dearith opened this issue Nov 6, 2020 · 1 comment
Open

AcquisitionBase - missing 'step_name' in constructor __init__ #8

dearith opened this issue Nov 6, 2020 · 1 comment

Comments

@dearith
Copy link

dearith commented Nov 6, 2020

In Metwork 1.0, variable step_name has been wrongly removed from the constructor, so that when in some case (1), the _get_logger function failed :

File "/opt/metwork-mfext-1.0/opt/python3/lib/python3.7/site-packages/acquisition/base.py", line 153, in info
    logger = self._get_logger()
  File "/opt/metwork-mfext-1.0/opt/python3/lib/python3.7/site-packages/acquisition/base.py", line 129, in _get_logger
    self.step_name,
AttributeError: 'AmqpSubscriber' object has no attribute 'step_name'

step_name is only defined in _init function which is called by run function

I 'm using a plugin (AmqpSubscriber class) which inherits from AcquisitionListener (almost same as Metwork listener).

(1) when I run my units tests, so the run function of AcquisitionListener is not called ad also the _init and step_name is not defined ... and my unit tests failed

AcquisitionBase Metwork 1.0

class AcquisitionBase(object):
    """Abstract class to describe an acquisition base.

    You have to override this class.

    Attributes:
        unit_tests (boolean): if True, we are in unit tests mode.
        unit_tests_args (string): cli args (for unit tests).

    """
    unit_tests = False
    unit_tests_args = None

    def __init__(self):
        """Constructor."""
        self.plugin_name = os.environ.get("MFDATA_CURRENT_PLUGIN_NAME",
                                          None)
        if self.plugin_name is None:
            if self.unit_tests:
                self.plugin_name = "unittests"
            else:
                raise Exception("you have to execute this inside a plugin_env")
        validate_plugin_name(self.plugin_name)
        self.args = None
        self.__logger = None

    def _init(self):
        description = "%s/%s acquisition step" % (
            self.plugin_name,
            self.__class__.__name__,
        )
        parser = argparse.ArgumentParser(description=description)
        parser.add_argument("--step-name", action="store", default="main",
                            help="step name")
        self._add_extra_arguments_before(parser)

AcquisitionBase Metwork 0.9

class AcquisitionBase(object):
    """Abstract class to describe an acquisition base.

    You have to override this class.

    Attributes:
        args (Namespace): argparser Namespace object with parsed cli args.
        unit_tests (boolean): if True, we are in unit tests mode.
        unit_tests_args (string): cli args (for unit tests).
        __logger (Logger): Logger object.
        plugin_name (string): the name of the plugin.
        step_name (string): the name of the step (if you inherits from
            AcquisitionStep).
        daemon_name (string): the name of the daemon (if you inherits from
            AcquisitionListener).

    """

    args = None
    unit_tests = False
    unit_tests_args = None
    __logger = None
    plugin_name = None
    step_name = "main"  # default value
    daemon_name = None

    def __init__(self):
        """Constructor."""
        if self.plugin_name is None:
            raise NotImplementedError("plugin_name property must be overriden "
                                      "and set")
        step_or_daemon_name = self._get_step_or_daemon_name()
        plugin_name = self.plugin_name
        regexp = "^[A-Za-z0-9_]+$"
        if not re.match(regexp, step_or_daemon_name):
            self.error_and_die(
...

So, it would be nice to add self.step_main = None in the ctor __init__ of the AcquisitionBase class. What do you think about ?
I think the 'well done' way in python, is to declare class variables in the ctor and this prevents me from declaring step_name in my own class to avoid issues. Since AcquisitionListener is a daemon, step_name is not used in this case, I think.

@thefab
Copy link
Member

thefab commented Jun 18, 2021

reverted and reopened as it breaks things for other people

@thefab thefab reopened this Jun 18, 2021
thefab added a commit to metwork-framework/mfext that referenced this issue Jun 23, 2021
mergify bot pushed a commit to metwork-framework/mfext that referenced this issue Jun 23, 2021
mergify bot pushed a commit to metwork-framework/mfext that referenced this issue Jun 23, 2021
See metwork-framework/acquisition#8

(cherry picked from commit 91a5869)

# Conflicts:
#	.metwork-framework/components.md
#	layers/layer2_python3/0500_extra_python_packages/requirements3.txt
mergify bot pushed a commit to metwork-framework/mfext that referenced this issue Jun 23, 2021
See metwork-framework/acquisition#8

(cherry picked from commit 91a5869)

# Conflicts:
#	.metwork-framework/components.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants