Skip to content

Comments

Removed nested level of message in HttpTextNormalizer#194

Merged
dangerink merged 5 commits intosberdevices:mainfrom
justanotheruser:main
Feb 21, 2022
Merged

Removed nested level of message in HttpTextNormalizer#194
dangerink merged 5 commits intosberdevices:mainfrom
justanotheruser:main

Conversation

@justanotheruser
Copy link
Contributor

@justanotheruser justanotheruser commented Feb 1, 2022

Bug was created after changing _normalize_batch https://github.com/sberdevices/smart_app_framework/pull/189/files

@intsynko intsynko added the bugfix This pull request fix some error label Feb 4, 2022
Copy link
Collaborator

@intsynko intsynko left a comment

Choose a reason for hiding this comment

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

Не понял суть бага, потому что ПР, в котором он создан никак не менял ответ метода. Можешь объяснить?

P.S. понял суть бага, ответ действительно менялся

answer = batch[0]

if "message" not in answer:
raise NormalizationError(f"Malformed Message Received: {batch}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

орбработка таких ошибок куда то перенесена или совсем убрана? если совсем, то почему?

Copy link
Collaborator

Choose a reason for hiding this comment

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

в данном случае обработка вообще убрана и на уровне result = [item["message"] for item in response.json()] будет рейситься KeyError: key 'message' not found. Есть ли разница что будет рейситься в этом месте?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Если это действительно важно, то может обернуть так @justanotheruser:

try:
    result = [item["message"] for item in response.json()]
except KeyErrror:
    raise NormalizationError(f"Malformed Message Received: {batch}")

@intsynko intsynko linked an issue Feb 14, 2022 that may be closed by this pull request
@dangerink dangerink merged commit be28475 into sberdevices:main Feb 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugfix This pull request fix some error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Некорректная работа http_text_normalizer.py

5 participants