changes needed for structured logging in modules

[goodenou]

Channel name is added to 'parameters' dict in TaskManager, so that the Module class can instantiate a logger with the channel name, which can then be accessed by all types of child Modules (Transform, LogicEngine, etc)

[pep8speaks]

Hello @goodenou! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-08-25 19:28:38 UTC

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 1c70ed9d2f53cef25f264219025f23b6337e5e35 into b9d28fbf7bb81ec1ab18976b15fc743311cf49d0 - view on LGTM.com

new alerts:

• 1 for Unused import

[mambelli]

There are also single quotes 'test' while we normally use double. Anyway, you can leave that, black will replace it.

[goodenou]

Done. Thanks Pat, you were right.

On Aug 25, 2021, at 1:39 PM, Pat Riehecky @.***> wrote:

@jcpunk commented on this pull request.

In src/decisionengine/framework/taskmanager/TaskManager.py https://github.com/HEPCloud/decisionengine/pull/500#discussion_r696018982:

• if "channel_name" in config_dict["parameters"].keys():
• return class_type(config_dict["parameters"])
• else:
• return class_type(dict(config_dict["parameters"], channel_name=channel_name)) I believe you'll need return class_type(dict(**config_dict["parameters"], channel_name=channel_name)) instead

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HEPCloud/decisionengine/pull/500#pullrequestreview-738707929, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE44QBWKJWTWFUR3T743M23T6U2FRANCNFSM5CXRZYZA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

[jcpunk]

Looks like https://github.com/HEPCloud/decisionengine/blob/master/src/decisionengine/framework/tests/ModuleProgramOptions.py has a specific idea what the config output should look like for the parameters.

[goodenou]

Yeah, that is one I think I need help with.

On Aug 26, 2021, at 10:04 AM, Pat Riehecky @.***> wrote:

Looks like https://github.com/HEPCloud/decisionengine/blob/master/src/decisionengine/framework/tests/ModuleProgramOptions.py https://github.com/HEPCloud/decisionengine/blob/master/src/decisionengine/framework/tests/ModuleProgramOptions.py has a specific idea what the config output should look like

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HEPCloud/decisionengine/pull/500#issuecomment-906493234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE44QBXK5K4CZQHVEMORAELT6ZJYVANCNFSM5CXRZYZA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

[jcpunk]

I think this should do it:

diff --git a/src/decisionengine/framework/tests/ModuleProgramOptions.py b/src/decisionengine/framework/tests/ModuleProgramOptions.py
index 4acc414..f609c23 100644
@@ -157,8 +159,8 @@ def _expected_acquire_result(name, config_file=None, multiplier=1, channel_name=
         result += f"Running acquire for source {name} using default configuration:"
     else:
         result += f"Running acquire for source {name} using configuration from {config_file.name}:"
-    result += " {'channel_name': " + f"{channel_name}" + \
-              "} {'multiplier':  " + f"{multiplier}" + \
+    result += " {'channel_name': " + f"'{channel_name}'" + \
+              ", 'multiplier':  " + f"{multiplier}" + \
               "} Produced products: {'foo':      col1  col2 " + \
               f"0  value1   {0.5 * multiplier} " + \
               f"1  value2   {2.0 * multiplier}" + "}"