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.

Sign up for GitHub

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

Jump to bottom

[chart/navi-async-matrix] chart syncup #520

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

[chart/navi-async-matrix] chart syncup #520

wants to merge 1 commit into from

Conversation

uk-navi-ci
Copy link
Collaborator

@uk-navi-ci uk-navi-ci commented Oct 10, 2024 •
edited by golovanovsv
Loading

Pull Request description

Changelog

  • Updates to 1.12.1
  • Trucks routing supported
  • Archiver job required for DMA for Trucks

Breaking changes

  • Existing DBs need task status type updated, in case public schema used:
ALTER TYPE public."statusvalues" ADD VALUE 'ATTRACT_PUSHED';
ALTER TYPE public."statusvalues" ADD VALUE 'ATTRACT_READY';
ALTER TYPE public."statusvalues" ADD VALUE 'ATTRACT_PROCESSED';
ALTER TYPE public."statusvalues" ADD VALUE 'ONE_TO_MANY_PUSHED';
ALTER TYPE public."statusvalues" ADD VALUE 'ONE_TO_MANY_READY';
ALTER TYPE public."statusvalues" ADD VALUE 'MERGER_PUSHED';
ALTER TYPE public."statusvalues" ADD VALUE 'MERGER_IN_PROGRESS';

Check-list. Чек-лист код-ревью

  • Запрос на слияние в develop.
  • Есть описание к PR.
  • Указаны блокирующие изменения. Breaking-Changes
  • Соответствие кода принятому стилю
    • Описание настроек.
    • Именование настроек.
    • Дефолтные значения.
    • Стиль кода.
  • Работоспособность. Разворачивается на своем окружении из ветки PR.
    • Тест API через тесты helmfile-хуков или коллекций Postman.
  • Не осталось мусора от удаления каких-то параметров. Ищется поиском по проекту из ветки PR.
  • Отработка линтера на чарт из ветки PR. Пример: helm lint charts/search-api

@i-bogomazov i-bogomazov force-pushed the auto/chart/navi-async-matrix branch from cab4378 to 49569c7 Compare October 21, 2024 03:35
@i-bogomazov i-bogomazov force-pushed the auto/chart/navi-async-matrix branch from 49569c7 to fe726ad Compare October 30, 2024 07:52
@i-bogomazov i-bogomazov marked this pull request as ready for review October 30, 2024 07:53
@i-bogomazov i-bogomazov requested review from a team as code owners October 30, 2024 07:53
@i-bogomazov i-bogomazov changed the title WIP: [chart/navi-async-matrix] chart syncup [chart/navi-async-matrix] chart syncup Oct 30, 2024
@dbelyaev-nsk
Copy link
Contributor

задача на проверку https://jira.2gis.ru/browse/ONPREM-1245

@i-bogomazov i-bogomazov force-pushed the auto/chart/navi-async-matrix branch from fe726ad to 8feae5e Compare November 8, 2024 09:00
@golovanovsv
Copy link
Contributor

Breaking changes заявлены в описание к PR, но не перенесены в Breaking-Changes.md

golovanovsv
golovanovsv reviewed Nov 13, 2024
View reviewed changes
charts/navi-async-matrix/values.yaml
# @param kafka.archiveTopic Name of the topic for archiving tasks.
# @param kafka.attractTopic Name of the topic for for attract tasks results
# @param kafka.oneToManyTopic Name of the topic for oneToMany tasks results
# @param kafka.vrpStatusTopic Name of the topic for VRP service integration
Copy link
Contributor

Choose a reason for hiding this comment

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

Не понятно что такое vrp и как он применяется.

golovanovsv
golovanovsv reviewed Nov 13, 2024
View reviewed changes
charts/navi-async-matrix/values.yaml
# @section Multi-DC settings

# @param multiDc.enabled If multi-DC functionality enabled
# @param multiDc.location Primary DC identifier.
Copy link
Contributor

Choose a reason for hiding this comment

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

Не понятно что такое location и как он применяется.

@golovanovsv
Copy link
Contributor

Так-же надо где-то написать, что включение dm.archiver.enabled требует обязательной настройки rbac и serviceaccount, а иначе работать не будет.

@i-bogomazov
Copy link
Contributor

Так-же надо где-то написать, что включение dm.archiver.enabled требует обязательной настройки rbac и serviceaccount, а иначе работать не будет.

что значит "обязательной настройки"? это сильно зависит от окружения

@golovanovsv
Copy link
Contributor

что значит "обязательной настройки"? это сильно зависит от окружения

Если я сделаю:

dm:
  archiver:
    enabled: true

А блоки serviceAccount и rbac оставлю как есть то у меня всё заработает?

dbelyaev-nsk
dbelyaev-nsk reviewed Jan 13, 2025
View reviewed changes
charts/navi-async-matrix/values.yaml
Comment on lines 313 to 316
groupId: navi_async_matrix
statusTopic: status_topic
cancelTopic: cancel_topic
archiveTopic: archive_topic
Copy link
Contributor

Choose a reason for hiding this comment

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

нужно убрать дефолт и сделать обязательными параметрами

Suggested change
groupId: navi_async_matrix
statusTopic: status_topic
cancelTopic: cancel_topic
archiveTopic: archive_topic
groupId: ''
statusTopic: ''
cancelTopic: ''
archiveTopic: ''

dbelyaev-nsk
dbelyaev-nsk reviewed Jan 13, 2025
View reviewed changes
charts/navi-async-matrix/values.yaml
@@ -289,10 +332,16 @@ kafka:
fileProperties: {}
taskTopicRules:
- topic: task_topic
type: car
Copy link
Contributor

Choose a reason for hiding this comment

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

разве тут можно указать type: truck?

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

@i-bogomazov i-bogomazov i-bogomazov left review comments

@dbelyaev-nsk dbelyaev-nsk dbelyaev-nsk left review comments

@golovanovsv golovanovsv golovanovsv left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@uk-navi-ci @dbelyaev-nsk @golovanovsv @i-bogomazov