OpenFn / kit

The bits & pieces that make OpenFn work. (diagrammer, cli, compiler, runtime, runtime manager, logger, etc.)
8 stars 12 forks source link

Fixing:#655 #690

Closed SatyamMattoo closed 1 month ago

SatyamMattoo commented 1 month ago

Short Description

Adding silent mode to the SanitizePolicy in order to disable all logs. Created a silent function to return null when silent mode is enabled.

Related issue

Fixes #655

QA Notes

Added tests for the silent mode testing an object and an array.

Checklist before requesting a review

SatyamMattoo commented 1 month ago

Hello @josephjclark,

I apologize for the oversight in my testing process. It appears that my tests only covered the cases I introduced, overlooking others. I will rectify this by reviewing and adjusting the changes accordingly.

If you believe this issue may be too complex for me, I am more than willing to redirect my efforts to other tasks better suited to me within the organization.

Thank you for your understanding and guidance.

Best regards

josephjclark commented 1 month ago

@SatyamMattoo no need to apologise!

Just remember to test the issue before you even start working on it so that you understand what you're trying to do. If you can't reproduce the problem, how would you expect to fix it?

It's not so much that the issue is complex, it's more that the solution isn't totally obvious to me. I welcome ideas - it again, you need to understand the problem first.

Always feel free to ask questions!

SatyamMattoo commented 1 month ago

I believe I have a potential solution to address this issue. A straightforward approach would involve adjusting the sanitization policy to 'silent', thereby allowing the function to return without executing further actions.

const log = (name: string | undefined, level: LogFns, ...args: LogArgs) => {
    if (priority[level] >= minLevel) {
    //if sanitization policy is silent it won't log anything
      if (options.sanitize != 'silent') {
        if (options.json) {
          logJSON(name, level, ...args);
        } else {
          logString(name, level, ...args);
        }
      }
    }
  };

Now where we set When configuring the logger as follows:

const logger = createLogger('x', { level: 'debug', sanitize: 'silent' });
logger.log('abc');

The resulting output would be:

{
  level: undefined,
  namespace: undefined,
  icon: undefined,
  message: ''
}

This behavior appears to match the expected outcome, effectively disabling logging functionality.

josephjclark commented 1 month ago

Hi @SatyamMattoo,

I don't think this will help. We already have a log level of none which will do what we want (which is stop all logs).

I don't know if you saw but I left more notes in the original issue. This actually has nothing to do with sanitisation policies.

I'll have a look at this issues this morning and see if can find something more suitable for you.

josephjclark commented 1 month ago

Hi @SatyamMattoo I've updated the list of Good First Issues - perhaps you can find something in there to work on?

I've tried to fill out the requirements but I expect you will have questions - please submit them in the issue

SatyamMattoo commented 1 month ago

I will surely check them out. Thank you so much.