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

Добавлен параметр для обновления расширения из хранилища #310

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

Conversation

Borisskin
Copy link
Contributor

Не хватало параметра вызова, который уже есть в МенеджерКонфигуратора.ЗапуститьОбновлениеИзХранилища
При вызове runner loadrepo в cmd добавить параметр вызова, где NameExtension - имя расширения
call runner loadrepo --storage-ext "-Extension \"NameExtension\""

Добавлен "--storage-ext", который будет использован для доп.ключей запуска обновления из хранилища
Добавлен "--storage-ext", который используется как доп.ключи запуска обновления из хранилища. Для возможности обновить расширение.
Copy link
Collaborator

@artbear artbear left a comment

Choose a reason for hiding this comment

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

немного бы дополнить

@@ -31,6 +31,7 @@
Парсер.ДобавитьИменованныйПараметрКоманды(ОписаниеКоманды, "--storage-pwd", "Пароль");
Парсер.ДобавитьИменованныйПараметрКоманды(ОписаниеКоманды, "--storage-ver",
"Номер версии, по умолчанию берем последнюю");
Парсер.ДобавитьИменованныйПараметрКоманды(ОписаниеКоманды, "--storage-ext", "Доп. ключи запуска");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Предлагаю еще добавить спец. параметр "--extension" для указания имени расширения

и прокидывать его, передавая как еще один доп.параметр.

т.е. будет 2 новых параметра у команды.

в итоге код будет такой

ДопПараметры = СтрШаблон("%1 %2", ПараметрыКоманды["--extension"], ПараметрыКоманды["--storage-ext"]);
МенеджерКонфигуратора.ЗапуститьОбновлениеИзХранилища(
			ПараметрыКоманды["--storage-name"], ПараметрыКоманды["--storage-user"],
			ПараметрыКоманды["--storage-pwd"], 
			ПараметрыКоманды["--storage-ver"]);
			ПараметрыКоманды["--storage-ver"], ДопПараметры);

Copy link
Collaborator

Choose a reason for hiding this comment

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

И получим простую команду
call runner loadrepo --extension "NameExtension"

Copy link
Contributor Author

@Borisskin Borisskin Aug 30, 2019

Choose a reason for hiding this comment

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

Чтобы сработало указание на расширение, необходимо в строке запуска из cmd добавить
-Extension "NameExtension"

Предлагаемый вариант с параметром --extension ведёт к тому, что надо дополнительно изменить код в МенеджерКонфигуратора.ЗапуститьОбновлениеИзХранилища и там обрабатывать параметр, и если он не пустой - добавлять к строке запуска "-Extension"
Или я не так понял предлагаемый вариант?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Сейчас столкнулся с тем, что в КомандаОтключитьсяОтХранилища.os тоже не хватает параметра для указания расширения, как и для КомандаПодключитьсяКХранилищу.os.
И теперь согласен с тем, что надо вводить просто параметр для имени расширения.

Почему-то в других *.os параметр имени расширения объявлен как 'extensionName', без двойного дефиса в начале. Привести бы к общему объявлению параметра.

Изменен код ПодключитьсяКХранилищу - чтобы можно было использовать дополнительные ключи запуска
Изменен код ОтключитьсяОтХранилища - проверка на дополнительные ключи запуска и изменение текста лога
Введен именованный параметр для имени хранилища
Изменен код для использования расширения
Добавлен параметр команды для передачи имени расширения
Замена переменной для имени расширения --extension
@artbear artbear modified the milestones: next, 1.9.0 Oct 31, 2019
Copy link
Collaborator

@artbear artbear left a comment

Choose a reason for hiding this comment

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

есть много вопросов для обсуждения.
Обсудим/исправим?

Парсер.ДобавитьПозиционныйПараметрКоманды(ОписаниеКоманды, "Пароль", "Пароль пользователя хранилища 1С");


Парсер.ДобавитьИменованныйПараметрКоманды(ОписаниеКоманды, "--storage-name", "Строка подключения к хранилищу");
Copy link
Collaborator

Choose a reason for hiding this comment

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

вот эти изменения поломают существующие команды и CI (

т.к. уходим от позиционных параметров к указанию имен.

Copy link
Collaborator

Choose a reason for hiding this comment

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

пока предлагаю параметры логина и пароля оставить позиционными, а имя расширения указать как именованный параметр.

Параметры.Добавить("/ConfigurationRepositoryUnbindCfg");
Параметры.Добавить("-force");
Параметры.Добавить("/ConfigurationRepositoryUnbindCfg ");
Параметры.Добавить("-force ");
Copy link
Collaborator

Choose a reason for hiding this comment

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

зачем нужны доп.пробелы в конце 2х ключей ??

  • "/ConfigurationRepositoryUnbindCfg "
  • "-force "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Тут перемудрил и пробелы, действительно, лишние

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Почему в МенеджерКонфигуратора.ПодключитьсяКХранилищу для подключения используется МенеджерХранилищаКонфигурации, а не сразу УправлениеКонфигуратором?
Первый не принимает имя расширения, его дописывать надо. Во второй же просто добавить параметр на выполнение.
Как раз из-за CI?


Если Не ПустаяСтрока(ДополнительныеКлючиЗапуска) Тогда
Параметры.Добавить(ДополнительныеКлючиЗапуска);
Лог.Информация("Выполняю отключение от хранилища расширения");
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.

и здесь считаю неверной проверку на хранилище расширения выполнять через
Если Не ПустаяСтрока(ДополнительныеКлючиЗапуска)

закладываем мину на будущее.
если в будущем добавятся другие доп.ключи, это условие будет неверным.

ДопПараметры = "";
ИмяРасширения = ПараметрыКоманды["--extension"];
Если Не ПустаяСтрока(ИмяРасширения) Тогда
ДопПараметры = "-Extension """+ИмяРасширения+"""";
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
Contributor Author

@Borisskin Borisskin Dec 18, 2019

Choose a reason for hiding this comment

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

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

  • МенеджерКонфигуратора.ОтключитьсяОтХранилища
  • МенеджерКонфигуратора.ПодключитьсяКХранилищу
  • МенеджерКонфигуратора.ЗапуститьОбновлениеИзХранилища

Либо, когда в командах встречается параметр ИмяРасширения, то можно вызывать в МенеджерКонфигуратора метод для Расширения. Ты это имел в виду?

@artbear artbear modified the milestones: 1.9.0, 1.10.0 Dec 13, 2019
@artbear
Copy link
Collaborator

artbear commented Dec 13, 2019

В общем, давай обсудим, исправим и включим в следующий релиз.

в релиз 1.9.0 не вошло из-за вопросов (

@artbear
Copy link
Collaborator

artbear commented Dec 17, 2019

@Borisskin обсудим?

@Borisskin
Copy link
Contributor Author

Borisskin commented Dec 17, 2019 via email

@Smilebringer
Copy link

Привет, еще придется модифицировать команду обновления БД. Ловлю такой момент, что при вызове updatedb не обновляет расширение. Думаю связано с тем, что без явного указания имени расширения обновляется только основная конфигурация

@Smilebringer
Copy link

Smilebringer commented Dec 17, 2019

Отбой, поторопился, платформенная бага
https://bugboard.v8.1c.ru/error/000046991

Изменение команд под параметр ИмяРасширения
Добавление параметра ИмяРасширения в вызов команды. Убрал из доппараметров
Добавление параметра ИмяРасширения в вызов команды. Убрал из доппараметров
Добавление параметра ИмяРасширения в вызов команды. Убрал из доппараметров.
@PavelDv
Copy link

PavelDv commented Jan 8, 2020

МенеджерКонфигуратора.os, строка 559
Переменная логинАдминистратора не объявлена. Просто Логин.

Старый код в 559. Вместо Логин было ЛогинАдминистратора
@artbear
Copy link
Collaborator

artbear commented Feb 29, 2020

@Borisskin сейчас в твоем ПР возник конфликт кода.

1 исправишь?
2 можешь добавить простое минимальное хранилище расширения

  • в нем 2 коммита для расширения
  • расширение простейшее для базы Ванесса-АДД (83Sync) - добавлен новый отчет, например

а я бы на основании этого расширения сделал бы фичи для проверки твоего кода

хочется включить твой ПР в текущий релиз, как и обещал )
очень жду.

@Borisskin
Copy link
Contributor Author

На этой неделе сильно занят, если не получится, то на следующей точно сделаю.

@Borisskin
Copy link
Contributor Author

Borisskin commented Jul 30, 2020

Автоматом коммит сюда, а не в #391.
Не сверял, но параметр расширения можно для многих ключей реализовать: обсуждение на инфостарт

@artbear artbear modified the milestones: 1.11.0, 1.12.0 Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants