remove LUIS-specific validation logic for Orchestrator scenarios #1224
Conversation
|
Would it be possible to add large import as a test? |
We can create a unit test without loading a large LU file, but by dynamically creating on the fly. Will create a unit test. |
| throw error; | ||
| } | ||
| public static debuggingThrow1WithCause( | ||
| message: any, |
There was a problem hiding this comment.
Please avoid code duplication. You can change this to a list/array of objectArgument.
…RemoveLuisValidationLogic
…RemoveLuisValidationLogic
| this.name = this.constructor.name; | ||
| } | ||
|
|
||
| public getCause(): Error { |
There was a problem hiding this comment.
Why not public readonly cause?
| @@ -25,6 +27,8 @@ export class Utility { | |||
| public static toPrintDebuggingLogToConsole: boolean = false; | |||
| public static toPrintDetailedDebuggingLogToConsole: boolean = false; | |||
|
|
|||
| public static externalLoggingObject: any = null; | |||
There was a problem hiding this comment.
unknown would be better than any, if possible.
| } | ||
| return logMessage; | ||
| } | ||
| public static debuggingLog2WithCause( |
There was a problem hiding this comment.
These all could likely be collapsed into a single method with a rest param. Something like
public static debuggingLogWithCause(message: string, cause: Error, ...rest: unknown[]) {
const isWarning = typeof rest[rest.length - 1] === 'boolean' ? rest.pop() : false;
...
}| @@ -2289,62 +2300,334 @@ export class Utility { | |||
| Utility.toObfuscateLabelTextInReportUtility = flag; | |||
| } | |||
|
|
|||
| public static debuggingLog( | |||
| public static resetExternalLoggingObject(externalLoggingObject: any) { | |||
| Utility.externalLoggingObject = externalLoggingObject; | |||
There was a problem hiding this comment.
Having static, shared state is a little worrying. What about concurrent execution?
…RemoveLuisValidationLogic
The lu parser is designed for LUIS which imposes a constraint on the number of utterances. For Orchestrator, it can handle more utterances and this limitation needs to be removed.
This will not impact existing LUIS validations, worked with @munozemilio to use alternate implementation that doesn't impose limit.
We have a customer (ATOS) who will run into this sooner than later.
#1223