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

Tolerant null in LG context #3329

Merged
merged 6 commits into from
Feb 6, 2020
Merged

Tolerant null in LG context #3329

merged 6 commits into from
Feb 6, 2020

Conversation

Danieladu
Copy link
Collaborator

@Danieladu Danieladu commented Feb 3, 2020

close: #3326

Original issue:

# hello
- @{user.name}

Today if user.name is null, it will result an exception, we should change this to "null" (we can optimize the result later) so that user won't be interrupted by exceptions.

Expected result:

>if a == null or a.b == null, result would be string "null"
# template1
- @{a.b} 

> if a== null or a.b == null, result would be "result is 'null'"
# template2
- switch: @{a.b}
- CASE: @{'null'}
- result is 'null'

> if a template ref is return null, the result would be 
>{ "lgType":"MyStruct", "key1":"null"}
# template3
[MyStruct
    key1 = @{template4()}
]

> return null
# template4
- IF:@{false}
- hello 

@Danieladu Danieladu requested review from boydc2014 and chrimc62 and removed request for chrimc62 February 3, 2020 11:53
@coveralls
Copy link
Collaborator

coveralls commented Feb 3, 2020

Pull Request Test Coverage Report for Build 104114

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 17 unchanged lines in 10 files lost coverage.
  • Overall coverage increased (+0.03%) to 78.059%

Files with Coverage Reduction New Missed Lines %
/libraries/Microsoft.Bot.Builder.Dialogs/ObjectPath.cs 1 86.46%
/libraries/Microsoft.Bot.Builder.Dialogs/Prompts/OAuthPrompt.cs 1 56.4%
/libraries/Microsoft.Bot.Builder/Inspection/InspectionMiddleware.cs 1 78.89%
/libraries/Microsoft.Bot.Builder.LanguageGeneration/LGErrors.cs 1 55.56%
/libraries/Microsoft.Bot.Expressions/BuiltInFunctions.cs 1 93.21%
/libraries/Microsoft.Bot.Expressions/LRUCache.cs 1 82.93%
/libraries/Microsoft.Bot.Expressions/TriggerTrees/Trigger.cs 1 85.09%
/libraries/Microsoft.Bot.Builder.Azure/CosmosDbStorageOptions.cs 2 75.0%
/libraries/Microsoft.Bot.Builder/MemoryTranscriptStore.cs 2 86.09%
/libraries/Microsoft.Bot.Builder.LanguageGeneration/Evaluator.cs 6 92.83%
Totals Coverage Status
Change from base Build 104013: 0.03%
Covered Lines: 13530
Relevant Lines: 17333

💛 - Coveralls

@fuselabs
Copy link
Collaborator

fuselabs commented Feb 3, 2020

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.ApplicationInsights.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.AspNet.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Testing.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll compared against version 4.6.3

@Danieladu Danieladu marked this pull request as ready for review February 4, 2020 03:14
@xieofxie
Copy link
Contributor

xieofxie commented Feb 4, 2020

will this be a public option instead of a private one?
For example, when developing, report null with error and when in production, replace null with custom warning like: [Not Set] and also trace it in telemetry for further investigation.

@boydc2014
Copy link
Contributor

boydc2014 commented Feb 4, 2020

will this be a public option instead of a private one?
For example, when developing, report null with error and when in production, replace null with custom warning like: [Not Set] and also trace it in telemetry for further investigation.

This PR's intention is to make not throw exception as default. We will explore whether make it configurable with something like "strict=true", which is out of scope of this one, and will be taken into considerations with other possible configurable behavior together.

@fuselabs
Copy link
Collaborator

fuselabs commented Feb 4, 2020

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.ApplicationInsights.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.AspNet.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Testing.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll compared against version 4.6.3

@fuselabs
Copy link
Collaborator

fuselabs commented Feb 6, 2020

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.ApplicationInsights.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.AspNet.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Testing.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll compared against version 4.6.3

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

Successfully merging this pull request may close these issues.

Tolerant null in LG context
6 participants