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

Добавлено хранилище данных об ошибках на entity #24

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

oshabanov
Copy link

  • Добавлен класс ХранилищеБазыДанных (в appsettings выделен как dbase, тип СУБД управляется параметров type). Реализован на базе библиотеки entity. (Прим. тестировалось только на SQLite. Работает у меня на продакшене 2 недели без замечаний и проблем);
  • Как следствие, - сделал некоторый рефакторинг класса ФайловоеХранилище: разделил чтение/запись данных об ошибках и обработку ошибок. Обработка ошибок вынесена в модуль МенеджерХранилищаОшибок.
  • Изменил функции получения отпечатка ошибки. В получении отпечатка используется только appStackHash, если он присутствует в инфо об ошибке в запросе, иначе clientStackHash + serverStackHash. Это изменение обусловлено регистрацией дублей в трекере задач. При анализе проблемы - выяснилось, что одна и та же ошибка в коде может иметь одинаковый appStackHash, но отличаться clientStackHash. По моим наблюдениям - из-за разных версий ОС, либо разной разрядности клиентов 1С.
  • В провайдер Jira добавлен параметр term (см. appsettings). Это кол-во дней для выполнения задачи. Без срока окончания - у меня Jira категорически отказывалась создавать задачу

@ovcharenko-di
Copy link
Owner

@oshabanov @Shabanov-Oleg (вас двое?? 😀 )
спасибо за PR! Я пока только по диагонали пробежался, но мне понравилось то, что я увидел!

Надо не забыть указать в Release notes :

  • добавлена обработка userDescription

@oshabanov
Copy link
Author

Да, обе учетки мои :) Объяснять долго, проще сказать, что тек. - моя основная.
Про Release notes - не понял... Если можно - в двух словах куда надо тыкнуть, чтоб это указать...

Вижу, что все проверки провалились. Я правильно понял, что нужно допилить автотесты (папочка tests), чтоб проверки прошли? (автотесты на OScript - для меня пока неизведанная тема)

@ovcharenko-di
Copy link
Owner

@oshabanov про Release notes - это я больше для себя пометку оставил.

Да, про тесты все правильно, надо именно их исправить. Мне и самому было бы интересно это сделать, только вот времени не найдется.

@ovcharenko-di
Copy link
Owner

@oshabanov а ты локально тесты прогоняешь, все зеленое?

@oshabanov
Copy link
Author

Да, локально. Только что добил их до зеленых.
Модифицировал их под измененную архитектуру. Для нового класса ХранилищеБазДанных тестов не делал

@ovcharenko-di
Copy link
Owner

@oshabanov у меня при попытке отправить ошибку в Redmine возникает такое исключение:

image

посмотришь? хорошо бы еще тест на это написать

@oshabanov
Copy link
Author

@oshabanov у меня при попытке отправить ошибку в Redmine возникает такое исключение:

image

посмотришь? хорошо бы еще тест на это написать

Да, вечером гляну.
На одном и том же каталоге data экспериментируешь с разными трекерами?

@ovcharenko-di
Copy link
Owner

@oshabanov нет, только на Redmine

обнаружил еще один косяк: при неудачной попытке создать задачу сервис на следующий раз сообщает, что такая ошибка уже зарегистрирована (с пустым Ид).

на это тоже нужен тест + фикс, желательно, в таком же порядке

@oshabanov
Copy link
Author

@ovcharenko-di
Ошибку я пофиксил (пока не заливал в PR. Тесты еще в стадии разработки. Залью все вместе), но решение, как по мне - костылевое. Суть: из базы вначале выполняется чтение по ключу (ОтпечаткуОшибки), а затем - запись, вместо обычного добавления. Это нужно для случаев, когда запись об ошибке была добавлена в базу, но по какой-то причине не была передана в систему интеграции. Как результат - запись в базе с ключом = ОтпечаткуОшибки уже есть, и в таком случае пользователю в 1С хорошо бы было бы отображать сообщение что-то типа "Ваша ошибка уже зарегистрирована, но задача не создана. Обратитесь к администратору..."

Хорошим решением здесь была бы реализация вот этих задач: #19 и #21

@oshabanov
Copy link
Author

@ovcharenko-di
залил фикс ошибки. Добавил два теста. Все зеленые
image

@ovcharenko-di
Copy link
Owner

@oshabanov почему не принимаю MR:
Хотел опробовать изменения на одном из новых проектов, но в этом проекте используется Redmine и настроен трекер, в котором определены нетиповые поля и они обязательны для заполнения.

Пожалуй, не имеет смысла поддерживать гибкую настройку заполнения нетиповых полей на стороне reperr, т.к. это довольно непросто и избыточно. Проще, и, скорее всего, правильнее методологически - создавать в Redmine под задачи из reperr отдельный трекер со стандартным набором полей.

Как сделаю еще один подход - вернусь к MR

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.

3 participants