Сохранение ИБ в виде артефакта после выполнения всех шагов инциализации #149
Conversation
WalkthroughИзменения в проекте направлены на добавление новой функциональности архивации информационной базы (ИБ) с возможностью опционального сохранения в виде артефакта. Модифицированы несколько классов и функций для поддержки нового параметра Changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/ru/pulsar/jenkins/library/IStepExecutor.groovy (1)
92-93: Добавьте документацию для нового методаДля поддержания единообразия кодовой базы и облегчения дальнейшей поддержки, рекомендуется добавить документацию к новому методу zip.
+ /** + * Создает zip архив из указанной директории + * @param dir Директория для архивации + * @param zipFile Имя создаваемого архива + * @param glob Шаблон для выбора файлов + * @param archive Флаг сохранения архива как артефакта + */ def zip(String dir, String zipFile, String glob, boolean archive)src/ru/pulsar/jenkins/library/configuration/InitInfoBaseOptions.groovy (1)
43-45: Дополните документацию параметра archiveInfobaseТекущее описание недостаточно информативно. Рекомендуется добавить подробности о назначении параметра и его влиянии на процесс.
@JsonPropertyDescription(""" - Сохранить базу после выполнения всех шагов инициализации + Сохранить базу после выполнения всех шагов инициализации. + При значении true: + * База будет сохранена как артефакт сборки + * Артефакт будет доступен для скачивания после завершения пайплайна + По умолчанию: false """)src/ru/pulsar/jenkins/library/StepExecutor.groovy (1)
199-202: Реализация выглядит корректной!Метод корректно реализует интерфейс и делегирует вызов в steps.zip. Было бы полезно добавить документацию для параметра archive, описывающую его назначение.
@Override + /** + * @param archive если true, то архив будет сохранен как артефакт сборки + */ def zip(String dir, String zipFile, String glob = '', boolean archive) { steps.zip dir: dir, zipFile: zipFile, glob: glob, overwrite: true, archive: archive }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
resources/globalConfiguration.jsonis excluded by!**/*.jsonresources/schema.jsonis excluded by!**/*.json
📒 Files selected for processing (6)
src/ru/pulsar/jenkins/library/IStepExecutor.groovy(1 hunks)src/ru/pulsar/jenkins/library/StepExecutor.groovy(1 hunks)src/ru/pulsar/jenkins/library/configuration/InitInfoBaseOptions.groovy(2 hunks)src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy(1 hunks)vars/pipeline1C.groovy(1 hunks)vars/zipInfobase.groovy(1 hunks)
🔇 Additional comments (2)
src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy (1)
85-85: Проверьте совместимость с существующими отчетамиИзменение имени файла с
junit.xmlнаallure.xmlможет повлиять на существующие процессы и инструменты, которые ожидают файл с прежним именем.✅ Verification successful
Изменение безопасно - это дополнительный отчет
Изменение не затрагивает существующий отчет junit.xml. Файл allure.xml создается как дополнительный отчет в отдельной директории, в то время как оригинальный junit.xml продолжает генерироваться в своем местоположении.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Поиск использования старого имени файла в кодовой базе rg -l "junit\.xml" --type groovyLength of output: 85
Script:
#!/bin/bash # Показать контекст использования junit.xml в файле rg "junit\.xml" --type groovy -C 3Length of output: 694
vars/pipeline1C.groovy (1)
147-147: Изменение корректно, но требуется проверка таймаута!Передача конфигурации в метод
zipInfobaseпозволяет контролировать процесс архивации через параметры конфигурации. Убедитесь, что значение таймаутаtimeoutOptions.zipInfoBaseдостаточно для архивации больших информационных баз.Выполните следующий скрипт для проверки значения таймаута:
✅ Verification successful
Таймаут архивации настроен корректно!
Значение таймаута для операции архивации информационной базы установлено на 60 минут в глобальной конфигурации, что является достаточным для большинства случаев. При необходимости это значение может быть увеличено в конфигурации конкретной задачи.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Описание: Поиск определения таймаута для zipInfoBase # Ожидаемый результат: Найти значение таймаута и убедиться, что оно достаточно большое # Поиск определения timeoutOptions rg -A 5 "timeoutOptions.*zipInfoBase"Length of output: 517
Script:
#!/bin/bash # Search for timeoutOptions definition and values rg -g "!*.class" "timeoutOptions.*=|defaultTimeoutOptions" -A 5 # Also search for any yaml/json config files that might contain the timeout fd -e yaml -e yml -e json --exec grep -l "timeoutOptions\|zipInfoBase" {}Length of output: 960
Script:
#!/bin/bash # Check schema and configuration files for timeout values echo "=== Schema definition ===" cat resources/schema.json | jq '.properties.timeoutOptions' echo -e "\n=== Global configuration ===" cat resources/globalConfiguration.json | jq '.timeoutOptions' echo -e "\n=== Job configuration example ===" cat test/unit/resources/jobConfiguration.json | jq '.timeoutOptions'Length of output: 422
Script:
#!/bin/bash # Look for zipInfoBase in configuration files echo "=== Checking configuration files ===" rg "zipInfoBase" resources/ test/unit/resources/ # Check the JobConfiguration class for default values echo -e "\n=== Checking JobConfiguration class ===" cat src/ru/pulsar/jenkins/library/configuration/JobConfiguration.groovyLength of output: 5524
src/ru/pulsar/jenkins/library/configuration/InitInfoBaseOptions.groovy
Outdated
Show resolved
Hide resolved
ovcharenko-di
force-pushed
the
feature/save-dt
branch
from
4a82c66 to
f77ac27
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
vars/zipInfobase.groovy (2)
5-9:⚠️ Potential issueПроверьте входные параметры на null.
Необходимо добавить валидацию входных параметров во избежание NullPointerException.
def call(JobConfiguration config, String stageName) { + if (config == null) { + throw new IllegalArgumentException("Параметр config не может быть null") + } + if (stageName == null || stageName.trim().isEmpty()) { + throw new IllegalArgumentException("Параметр stageName не может быть null или пустым") + } ContextRegistry.registerDefaultContext(this)
8-9:⚠️ Potential issueДобавьте обработку ошибок.
Необходимо добавить try-catch блок для обработки возможных ошибок при выполнении.
- def zipInfobase = new ZipInfobase(config, stageName) - zipInfobase.run() + try { + def zipInfobase = new ZipInfobase(config, stageName) + zipInfobase.run() + } catch (Exception e) { + error "Ошибка при архивации информационной базы: ${e.message}" + }
🧹 Nitpick comments (4)
src/ru/pulsar/jenkins/library/configuration/ArchiveInfobaseOptions.groovy (1)
19-28: Рекомендуется добавить документацию к классуХотя реализация toString() корректна, было бы полезно добавить общее описание класса через JavaDoc, объясняющее его назначение и использование.
src/ru/pulsar/jenkins/library/steps/ZipInfobase.groovy (2)
30-35: Рефакторинг определения имени архиваРекомендуется вынести логику формирования имени архива в отдельный метод и избавиться от магических строк.
+ private static String getArchiveName(String stage) { + final String BASE_ARCHIVE_NAME = "1Cv8.1CD" + return stage == 'initInfoBase' + ? "${BASE_ARCHIVE_NAME}.zip" + : "${BASE_ARCHIVE_NAME}.${stage}.zip" + } + def run() { // ... - def archiveName - if (stage == 'initInfoBase') { - archiveName = "1Cv8.1CD.zip" - } else { - archiveName = "1Cv8.1CD.${stage}.zip" - } + def archiveName = getArchiveName(stage)
37-44: Упростите логику проверки условий архивацииТекущая реализация условий архивации может быть упрощена с использованием более компактного синтаксиса.
- def archiveInfobase = false - if (archiveInfobaseOptions.onAlways - || (archiveInfobaseOptions.onFailure && (currentResult == Result.FAILURE || currentResult == Result.ABORTED)) - || (archiveInfobaseOptions.onUnstable && currentResult == Result.UNSTABLE) - || (archiveInfobaseOptions.onSuccess && currentResult == Result.SUCCESS)) { - archiveInfobase = true - } + def archiveInfobase = archiveInfobaseOptions.onAlways + || (archiveInfobaseOptions.onFailure && (currentResult in [Result.FAILURE, Result.ABORTED])) + || (archiveInfobaseOptions.onUnstable && currentResult == Result.UNSTABLE) + || (archiveInfobaseOptions.onSuccess && currentResult == Result.SUCCESS)test/unit/groovy/ru/pulsar/jenkins/library/configuration/ConfigurationReaderTest.java (1)
68-68: Расширьте покрытие тестамиРекомендуется добавить проверки для всех свойств archiveInfobase (onSuccess, onFailure, onUnstable), а не только onAlways.
assertThat(jobConfiguration.getInitInfoBaseOptions().getArchiveInfobase().getOnAlways()).isTrue(); + assertThat(jobConfiguration.getInitInfoBaseOptions().getArchiveInfobase().getOnSuccess()).isFalse(); + assertThat(jobConfiguration.getInitInfoBaseOptions().getArchiveInfobase().getOnFailure()).isFalse(); + assertThat(jobConfiguration.getInitInfoBaseOptions().getArchiveInfobase().getOnUnstable()).isFalse();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
resources/globalConfiguration.jsonis excluded by!**/*.jsonresources/schema.jsonis excluded by!**/*.jsontest/unit/resources/jobConfiguration.jsonis excluded by!**/*.json
📒 Files selected for processing (12)
src/JobConfigurationSchemaGenerator.java(1 hunks)src/ru/pulsar/jenkins/library/IStepExecutor.groovy(1 hunks)src/ru/pulsar/jenkins/library/StepExecutor.groovy(1 hunks)src/ru/pulsar/jenkins/library/configuration/ArchiveInfobaseOptions.groovy(1 hunks)src/ru/pulsar/jenkins/library/configuration/BddOptions.groovy(1 hunks)src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy(2 hunks)src/ru/pulsar/jenkins/library/configuration/InitInfoBaseOptions.groovy(2 hunks)src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy(1 hunks)src/ru/pulsar/jenkins/library/steps/ZipInfobase.groovy(1 hunks)test/unit/groovy/ru/pulsar/jenkins/library/configuration/ConfigurationReaderTest.java(1 hunks)vars/pipeline1C.groovy(2 hunks)vars/zipInfobase.groovy(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/JobConfigurationSchemaGenerator.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/ru/pulsar/jenkins/library/IStepExecutor.groovy
- src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy
- src/ru/pulsar/jenkins/library/configuration/InitInfoBaseOptions.groovy
- src/ru/pulsar/jenkins/library/StepExecutor.groovy
🔇 Additional comments (5)
src/ru/pulsar/jenkins/library/configuration/ArchiveInfobaseOptions.groovy (1)
7-18: Структура класса выглядит правильной!Класс хорошо структурирован с четкими флагами для различных сценариев архивации. Описания свойств понятны и информативны.
src/ru/pulsar/jenkins/library/configuration/BddOptions.groovy (1)
19-22: Корректное добавление нового свойстваСвойство archiveInfobase добавлено правильно, с понятным описанием на русском языке.
src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy (1)
72-72: Добавление archiveInfobase в список nonMergeableSettings выглядит корректным.Настройка archiveInfobase не должна объединяться между конфигурациями, поэтому её добавление в список nonMergeableSettings является правильным решением.
vars/pipeline1C.groovy (2)
147-147: Обновление вызова zipInfobase выглядит корректным.Передача параметров
configи'initInfoBase'соответствует новой сигнатуре метода.
241-249: Добавление архивации в BDD этап реализовано корректно.Новый этап архивации:
- Имеет корректный таймаут
- Использует правильные параметры при вызове zipInfobase
- Расположен в логичном месте после выполнения BDD сценариев
src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy
Outdated
Show resolved
Hide resolved
| "archiveInfobase": { | ||
| "onAlways": false, | ||
| "onFailure": true, | ||
| "onUnstable": false, |
There was a problem hiding this comment.
тут возможно имеет смысл выставить в true, т.к. при падении тестов стейдж помечается как UNSTABLE, а не FAILED. ну или по крайней мере расписать это в документации.
There was a problem hiding this comment.
согласен, выставил true тут и в initInfobase
There was a problem hiding this comment.
я вот думаю, мы мастер не положим такими приколами, кстати? :)
может вырубить все по дефолту?
There was a problem hiding this comment.
вырубил
<поднятьворотачутьчуть.jpg>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
171-172: Уточните условия сохранения ИБ по умолчанию.Текущее описание поведения по умолчанию не полностью соответствует возможностям функционала. Предлагаю дополнить информацию о том, что это поведение можно изменить через параметры
onAlways,onFailure,onUnstable,onSuccess.- * На этапах `initInfobase` и `bdd` база сохраняется в виде артефакта при статусе `UNSTABLE` и `FAILURE` + * На этапах `initInfobase` и `bdd` база по умолчанию сохраняется в виде артефакта при статусах `UNSTABLE` и `FAILURE`. Это поведение можно изменить через параметры `onAlways`, `onFailure`, `onUnstable`, `onSuccess` в соответствующих секциях конфигурации.
318-325: Дополните документацию примером конфигурации.Для лучшего понимания функционала рекомендуется добавить пример конфигурации с различными вариантами настройки параметров сохранения ИБ.
* для шага `initInfoBase`: '1Cv8.1CD.zip' * для шага `bdd`: '1Cv8.1CD.bdd.zip' + +Пример конфигурации: + +```json +"initInfobase": { + "archiveInfobase": { + "onAlways": false, + "onFailure": true, + "onUnstable": true, + "onSuccess": false + } +}, +"bdd": { + "archiveInfobase": { + "onAlways": false, + "onFailure": true, + "onUnstable": true, + "onSuccess": false + } +} +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
resources/globalConfiguration.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
README.md(3 hunks)src/ru/pulsar/jenkins/library/steps/ZipInfobase.groovy(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ru/pulsar/jenkins/library/steps/ZipInfobase.groovy
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/ru/pulsar/jenkins/library/steps/ZipInfobase.groovy (2)
30-35: Вынесите имена архивов в константыСтроковые литералы "1Cv8.1CD.zip" и "1Cv8.1CD.${stage}.zip" лучше вынести в константы класса для улучшения поддерживаемости кода.
class ZipInfobase implements Serializable { + private static final String DEFAULT_ARCHIVE_NAME = "1Cv8.1CD.zip" + private static final String STAGE_ARCHIVE_PATTERN = "1Cv8.1CD.%s.zip" private final JobConfiguration config private final String stage
38-44: Упростите логику определения необходимости архивацииСложное условие лучше вынести в отдельный метод для улучшения читаемости.
+ private boolean shouldArchiveInfobase(ArchiveInfobaseOptions options, Result currentResult) { + return options.onAlways + || (options.onFailure && (currentResult == Result.FAILURE || currentResult == Result.ABORTED)) + || (options.onUnstable && currentResult == Result.UNSTABLE) + || (options.onSuccess && currentResult == Result.SUCCESS) + } + def run() { // ... - def archiveInfobase = false - if (archiveInfobaseOptions.onAlways - || (archiveInfobaseOptions.onFailure && (currentResult == Result.FAILURE || currentResult == Result.ABORTED)) - || (archiveInfobaseOptions.onUnstable && currentResult == Result.UNSTABLE) - || (archiveInfobaseOptions.onSuccess && currentResult == Result.SUCCESS)) { - archiveInfobase = true - } + def archiveInfobase = shouldArchiveInfobase(archiveInfobaseOptions, currentResult)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/ru/pulsar/jenkins/library/steps/ZipInfobase.groovy(1 hunks)
🔇 Additional comments (3)
src/ru/pulsar/jenkins/library/steps/ZipInfobase.groovy (3)
1-19: Структура класса реализована корректно!Класс правильно реализует Serializable для совместимости с Jenkins pipeline, имеет четкую структуру и необходимые зависимости.
53-67: Обработка ошибок реализована корректно!Метод правильно обрабатывает исключения и возвращает значения по умолчанию при возникновении ошибок. Предыдущие замечания по обработке ошибок были успешно исправлены.
49-50: Добавьте проверку существования директории build/ibПеред выполнением архивации следует проверить существование директории build/ib.
+ if (!steps.fileExists('build/ib')) { + throw new IllegalStateException("Директория build/ib не существует") + } steps.zip('build/ib', archiveName, '1Cv8.1CD', archiveInfobase) steps.stash(archiveName, archiveName, false)✅ Verification successful
Подтверждаю необходимость проверки существования директории build/ib
Предложенное изменение корректно, так как:
- Директория build/ib является критически важной для работы pipeline
- Ранняя проверка существования директории позволит получить более понятное сообщение об ошибке
- Изменение соответствует существующим практикам в кодовой базе
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for fileExists usage patterns ast-grep --pattern 'steps.fileExists($_)' # Search for build/ib references rg "build/ib" -B 2 -A 2 # Search for similar zip operations ast-grep --pattern 'steps.zip($$$)'Length of output: 8875
| @@ -237,6 +237,16 @@ void call() { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| stage('Архивация ИБ') { | |||
There was a problem hiding this comment.
нет ли смысла сразу везде сделать?
There was a problem hiding this comment.
думал об этом, но лично у меня дымовые и юниты выполняются в транзакции, поэтому сохранять ИБ после этих этапов нет смысла
я посчитал, что у большинства пользователей библиотеки ситуация аналогичная (могу ошибаться)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
317-324: Предлагаю улучшить документацию новой функциональности.Раздел содержит базовую информацию, но может быть улучшен для большей полезности пользователям.
Предлагаю добавить:
- Пример конфигурации в формате JSON
- Описание всех доступных статусов сборки
- Информацию о том, где искать сохраненные артефакты в интерфейсе Jenkins
## Сохранение ИБ в виде артефакта сборки Параметры `initInfobase` -> `archiveInfobase` и `bdd` -> `archiveInfobase` отвечают за сохранение информационной базы в виде артефакта сборки после выполнения соответствующих этапов. Можно управлять тем, при каких статусах сборки ИБ будет сохранена, см. `onAlways`, `onFailure`, `onUnstable`, `onSuccess`. Имя файла формируется следующим образом: * для шага `initInfoBase`: '1Cv8.1CD.zip' * для шага `bdd`: '1Cv8.1CD.bdd.zip' + +### Пример конфигурации + +```json +{ + "initInfobase": { + "archiveInfobase": { + "onFailure": true, + "onUnstable": true + } + }, + "bdd": { + "archiveInfobase": { + "onAlways": true + } + } +} +``` + +### Статусы сборки + +* `onAlways` - сохранять при любом статусе сборки +* `onFailure` - сохранять только при падении сборки +* `onUnstable` - сохранять при нестабильном статусе (например, при падении тестов) +* `onSuccess` - сохранять только при успешной сборке + +### Доступ к артефактам + +Сохраненные артефакты доступны в интерфейсе Jenkins на странице сборки во вкладке "Артефакты".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
resources/globalConfiguration.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
README.md(3 hunks)
🔇 Additional comments (2)
README.md (2)
54-54: Корректное добавление новой возможности в список.Добавление возможности сохранения информационной базы логично дополняет существующий список возможностей библиотеки.
171-171: Корректное указание поведения по умолчанию.Явное указание того, что по умолчанию информационная база не сохраняется, помогает пользователям понять базовое поведение системы.
fixes #139