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

Add ydb_types-credentials-login to Helpers component #349

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

popov-aa
Copy link

На текущий момент client-ydb_types-credentials-login не включен ни в один из компонентов, что усложняет использование, например в userver.
Добавил его в Helpers.

@Gazizonoki
Copy link
Collaborator

В Helpers добавлять не надо, предназначение этой компоненты в том, чтобы инициализировать CredentialsProvider на основе переменных окружения. Пока нет переменных для static аутентификации, значит и нет смысла добавлять туда. На мой взгляд, лучше пока сделать отдельный компонент Credentials, где будут все виды
аутентификации, кроме Iam. В будущем планируется рефакторинг, эти компоненты будут объединены в один

@Gazizonoki Gazizonoki self-requested a review November 27, 2024 16:34
@popov-aa
Copy link
Author

Спасибо за совет, сейчас реализую следуя ему.

@popov-aa
Copy link
Author

Сейчас все CredentialsProvider (за исключением Login) делятся по двум компонентам: Iam и Helpers.
Login остался сиротой, потому в новый компонент Credentials вошел он один.
Правильно ли я понял рекомендаю и сделать ли мне squash коммитов, или Вы сделаете это при слиянии, если все в порядке?

@Gazizonoki
Copy link
Collaborator

Я скорее думал добавить компоненту Credentials, которая будет содержать:

  • client-ydb_types-credentials
  • client-ydb_types-credentials-login
  • client-ydb_types-credentials-oauth2

Это чтобы можно было прилинковать сразу все провайдеры кроме Iam (он особенный, ходит в сторонний IAM сервис, хочется дать возможность пользователю полностью выпилить его, чтобы не пугать)

@popov-aa
Copy link
Author

_ydb_sdk_make_client_component создает алиас для указанного таргета.
Если мы создаем компоненту Credentials, необходимо указать для какой либы создается алиас. И у этой либы должны быть заданы зависимости для линковки: client-ydb_types-credentials, client-ydb_types-credentials-login, client-ydb_types-credentials-oauth2.
У нас нет такой либы)
В случае с компонентой Helpers, например, там из одного модуля src/client/helpers/helpers.cpp создается либа, client-helpers, для которой уже задаются зависимости линковки.
Чтобы объединить client-ydb_types-credentials, client-ydb_types-credentials-login, client-ydb_types-credentials-oauth2 в одной компоненте мне необходимо задать их в качестве зависимостей для какой-то либы, для которой и будет создан алиас.
Хотя для компиляции эта зависимость не будет необходимой, выглядит кривовато.

Либо моих знаний cmake не хватает, и я не понимаю, как создать виртуальный таргет, для которого можно будет задать зависимости линковки и алиас.

@popov-aa
Copy link
Author

popov-aa commented Dec 3, 2024

@Gazizonoki, я понял чего Вы от меня хотели) А слона то я и не заметил, у нас уже есть библиотека client-ydb_types-credentials,
Прописал для нее зависимости от client-ydb_types-credentials-login и client-ydb_types-credentials-oauth2 и добавил алиас для client-ydb_types-credentials.

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.

2 participants