Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Conversation

@VanyLaw
Copy link
Contributor

@VanyLaw VanyLaw commented Nov 12, 2019

Description

This PR is to fix this Issue

Root Cause
This Issue is because the botproject(runtime) read qna.endpointkey from disk before. After we omitted it, we need to send this from composer server to botproject(runtime).

Task Item

closes #1538

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code refactor (non-breaking change which improve code quality, clean up, add tests, etc)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Doc update (document update)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have functionally tested my change

Screenshots

During the QnaMaker trace, it won't show "Object not set to instance of an object" anymore.
image

@VanyLaw VanyLaw changed the title Wenyluo/fix Fix QnA Maker EndpointKey is null Issue Nov 12, 2019
@a-b-r-o-w-n a-b-r-o-w-n changed the title Fix QnA Maker EndpointKey is null Issue fix: QnA Maker EndpointKey is null Issue Nov 12, 2019
@cwhitten
Copy link
Member

The design here is odd. What happens when there are multiple QnA kb's in play? This looks to hardcode the qna key on the settings object.

Copy link
Member

@cwhitten cwhitten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See earlier comment

@VanyLaw
Copy link
Contributor Author

VanyLaw commented Nov 13, 2019

The design here is odd. What happens when there are multiple QnA kb's in play? This looks to hardcode the qna key on the settings object.

Yes, Thanks for the reminding. I don't have an accurate concept of multiple QnA kb's before. I treat it as a fixed property as luis key. According to @luhan2017 and @hibrenda 's description, I try some corner cases and find it has nothing to do with the property name( no mater it's qna.knowledgebaseid /qna.endpointkey/… or not). So those key-value pairs are more like self-defined property, user who want to use QnA just need to make sure the property in settings is the same as in form.
So, I think it's hard to find a pattern to omit those sensitive properties(such like endpointkey) saving to disk. The best solution for this Issue is simply treated it as self-defined value and not omit it.

FYI. It's a case that works.
image
image

@VanyLaw VanyLaw closed this Nov 13, 2019
@VanyLaw VanyLaw deleted the wenyluo/fix branch November 13, 2019 13:59
@VanyLaw
Copy link
Contributor Author

VanyLaw commented Nov 13, 2019

@cwhitten Sorry, I mis-deleted my branch caused this PR closed. I send a new one here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants