Support YDB in YQL Generic Provider (YQv1) #2300
Conversation
0c0b8a4 to
8d315c6
Compare
|
⚪ |
|
⚪ |
8d315c6 to
4ea12aa
Compare
|
⚪
|
|
⚪
|
| if (db.endpoint()) | ||
| clusterCfg->SetEndpoint(db.endpoint()); | ||
| clusterCfg->SetSecure(db.secure()); | ||
| clusterCfg->SetAddBearerToToken(common.GetUseBearerForYdb()); |
There was a problem hiding this comment.
Bearer - важная тема. хорошо бы ее тоже запилить. это нужно для работы внутри Я не с помощью OAuth токена, а с помощью IAM токена
There was a problem hiding this comment.
Ты имеешь в виду, добавить опцию AddBearerToToken вот сюда https://github.com/ydb-platform/ydb/blob/main/ydb/library/yql/providers/common/proto/gateways_config.proto#L547-L590, чтобы индивидуально указывать каждому кластеру?
Сейчас при ресолвинге безусловно true. Это неправильно? https://github.com/ydb-platform/ydb/blob/main/ydb/library/yql/providers/generic/provider/yql_generic_settings.cpp#L52
There was a problem hiding this comment.
пусть будет true, нормуль
There was a problem hiding this comment.
Ещё раз хочу по пунктам проговорить, что надо делать с bearer, потому что логика весьма запутанная и есть разные контексты применения bearer.
Поход в MDB API, в YDB CP (за ресолвингом)
Bearer всегда true. Так всё и остаётся.
Поход в YDB CP (за метаданными/данными)
Сейчас Bearer всегда false. Надо:
- Добавить в
TGenericClusterConfigполеAddBearerToToken. - Устанавливать значение из этого поля в YQv1 в
clusters_from_connections.cpp, брать его из конфига YQ. - Устанавливать значение из этого поля в YQv2 в external source factory, и оттуда прокидывать в провадйер через
TGenericProvider::AddCluster. Это означает, чтоAddBearerToTokenпросочится в DSL (как минимум в параметрыCREATE EXTERNAL DATA SOURCEдля YDB). - В конфигах YQ типа этого поменять true на false.
Всё верно?
There was a problem hiding this comment.
вроде верно. это доп флаг для аутентификации во внешней системе
ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp
Outdated
Show resolved
Hide resolved
ydb/library/yql/providers/generic/provider/yql_generic_load_meta.cpp
Outdated
Show resolved
Hide resolved
ydb/library/yql/providers/generic/provider/yql_generic_cluster_config.cpp
Outdated
Show resolved
Hide resolved
ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp
Outdated
Show resolved
Hide resolved
ydb/library/yql/providers/generic/actors/yql_generic_read_actor.cpp
Outdated
Show resolved
Hide resolved
| @@ -327,9 +333,11 @@ class TDatabaseResolver: public TActor<TDatabaseResolver> | |||
| if (!isDedicatedDb && ret.Endpoint.StartsWith("ydb.")) { | |||
| // Replace "ydb." -> "yds." | |||
| ret.Endpoint[2] = 's'; | |||
| ret.Host[2] = 's'; | |||
There was a problem hiding this comment.
избыточность. нет смысл заменить endpoint на два поля host + port ?
There was a problem hiding this comment.
Такие мысли были, но думаю, что точно не в этом ревью, так как это изменение затронет много провайдеров в yql.
4ea12aa to
16aa7a9
Compare
|
⚪
|
|
⚪
|
|
⚪ |
8616d9d to
b041211
Compare
|
⚪
|
|
⚪
|
|
⚪
|
|
⚪
|
* Support YDB in YQL Generic Provider (for YQv1 only) * Add `--token-accessor-endpoint` flag to dqrun * Drop some outdated tests
* Support YDB in YQL Generic Provider (for YQv1 only) * Add `--token-accessor-endpoint` flag to dqrun * Drop some outdated tests
* Support YDB in YQL Generic Provider (for YQv1 only) * Add `--token-accessor-endpoint` flag to dqrun * Drop some outdated tests
Changelog entry
--token-accessor-endpointflag todqrunChangelog category