Skip to content

fix(QueryEditor): pass base64 from settings #3093

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

Merged
merged 1 commit into from
Nov 18, 2025
Merged

fix(QueryEditor): pass base64 from settings #3093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/containers/Tenant/Query/QueryEditor/QueryEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ export default function QueryEditor(props: QueryEditorProps) {
const [lastExecutedQueryText, setLastExecutedQueryText] = React.useState<string>('');
const [isQueryStreamingEnabled] = useQueryStreamingSetting();

const [binaryDataInPlainTextDisplay] = useSetting<boolean>(
SETTING_KEYS.BINARY_DATA_IN_PLAIN_TEXT_DISPLAY,
);

const encodeTextWithBase64 = !binaryDataInPlainTextDisplay;

const isStreamingEnabled =
useStreamingAvailable() &&
isQueryStreamingEnabled &&
Expand Down Expand Up @@ -160,6 +166,7 @@ export default function QueryEditor(props: QueryEditorProps) {
database,
querySettings,
enableTracingLevel,
base64: encodeTextWithBase64,
});

queryManagerInstance.registerQuery(query);
Expand All @@ -172,6 +179,7 @@ export default function QueryEditor(props: QueryEditorProps) {
querySettings,
enableTracingLevel,
queryId,
base64: encodeTextWithBase64,
});

queryManagerInstance.registerQuery(query);
Expand Down Expand Up @@ -212,6 +220,7 @@ export default function QueryEditor(props: QueryEditorProps) {
querySettings,
enableTracingLevel,
queryId,
base64: encodeTextWithBase64,
});

queryManagerInstance.registerQuery(query);
Expand Down
6 changes: 2 additions & 4 deletions src/services/api/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import type {AxiosWrapperOptions} from '@gravity-ui/axios-wrapper';
import axiosRetry from 'axios-retry';

import {backend as BACKEND, clusterName} from '../../store';
import {readSettingValueFromLS} from '../../store/reducers/settings/utils';
import type {SchemaPathParam} from '../../types/api/common';
import {DEV_ENABLE_TRACING_FOR_ALL_REQUESTS} from '../../utils/constants';
import {prepareBackendWithMetaProxy} from '../../utils/parseBalancer';
import {isRedirectToAuth} from '../../utils/response';
import {settingsManager} from '../settings';

export type AxiosOptions = {
concurrentId?: string;
Expand Down Expand Up @@ -42,9 +42,7 @@ export class BaseYdbAPI extends AxiosWrapper {
// Make possible manually enable tracing for all requests
// For development purposes
this._axios.interceptors.request.use(function (config) {
const enableTracing = settingsManager.readUserSettingsValue(
DEV_ENABLE_TRACING_FOR_ALL_REQUESTS,
);
const enableTracing = readSettingValueFromLS(DEV_ENABLE_TRACING_FOR_ALL_REQUESTS);
Copy link
Collaborator

@astandrik astandrik Nov 18, 2025

Choose a reason for hiding this comment

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

Now we have two options to get setting value - via readSettingValueFromLS or via useSetting

Now its not that obvious (for me ofc) what method when to use.

What do you think of it? Could useSetting consider itself where to take setting value from?

Maybe its a bad idea - just a proposal for consideration.

Copy link
Member Author

@artemmufazalov artemmufazalov Nov 18, 2025

Choose a reason for hiding this comment

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

TLDR: currently for all settings you should use useSetting from utils/hooks.

Tracing is a dev setting, it should never be synced with external storage. So I read it directly from localStorage.

All other settings should be used and manipulated with useSetting, it will use local storage or meta settings service depending on app configuration and user auth.

In this PR I use old useSetting, new approach breaks tests, so I try separate all refactoring to reduce places where things can go wrong. Probably I should have named new hook differently to reduce ambiguity, but I had false hope that it will work from the beginning and I could just remove old code. So now we have two useSetting hook, you should use old hook from utils while new useSetting is not proved to work properly in all cases


if (enableTracing) {
config.headers['X-Want-Trace'] = 1;
Expand Down
12 changes: 3 additions & 9 deletions src/services/api/streaming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
isSessionChunk,
isStreamDataChunk,
} from '../../store/reducers/query/utils';
import {SETTING_KEYS} from '../../store/reducers/settings/constants';
import {readSettingValueFromLS} from '../../store/reducers/settings/utils';
import type {Actions, StreamQueryParams} from '../../types/api/query';
import type {
QueryResponseChunk,
Expand All @@ -18,7 +18,6 @@ import type {
} from '../../types/store/streaming';
import {DEV_ENABLE_TRACING_FOR_ALL_REQUESTS} from '../../utils/constants';
import {isRedirectToAuth} from '../../utils/response';
import {settingsManager} from '../settings';

import {BaseYdbAPI} from './base';

Expand All @@ -42,10 +41,7 @@ export class StreamingAPI extends BaseYdbAPI {
params: StreamQueryParams<Action>,
options: StreamQueryOptions,
) {
const base64 = !settingsManager.readUserSettingsValue(
SETTING_KEYS.BINARY_DATA_IN_PLAIN_TEXT_DISPLAY,
true,
);
const base64 = params.base64;

const queryParams = qs.stringify(
{timeout: params.timeout, base64, schema: 'multipart'},
Expand All @@ -66,9 +62,7 @@ export class StreamingAPI extends BaseYdbAPI {
headers.set('X-Trace-Verbosity', String(params.tracingLevel));
}

const enableTracing = settingsManager.readUserSettingsValue(
DEV_ENABLE_TRACING_FOR_ALL_REQUESTS,
);
const enableTracing = readSettingValueFromLS(DEV_ENABLE_TRACING_FOR_ALL_REQUESTS);

if (enableTracing) {
headers.set('X-Want-Trace', '1');
Expand Down
7 changes: 1 addition & 6 deletions src/services/api/viewer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type {PlanToSvgQueryParams} from '../../store/reducers/planToSvg';
import {SETTING_KEYS} from '../../store/reducers/settings/constants';
import type {VDiskBlobIndexStatParams} from '../../store/reducers/vdisk/vdisk';
import type {
AccessRightsUpdateRequest,
Expand Down Expand Up @@ -40,7 +39,6 @@ import type {VDiskBlobIndexResponse} from '../../types/api/vdiskBlobIndex';
import type {TUserToken} from '../../types/api/whoami';
import type {TabletsApiRequestParams} from '../../types/store/tablets';
import type {Nullable} from '../../utils/typecheckers';
import {settingsManager} from '../settings';

import type {AxiosOptions} from './base';
import {BaseYdbAPI} from './base';
Expand Down Expand Up @@ -416,10 +414,7 @@ export class ViewerAPI extends BaseYdbAPI {
params: SendQueryParams<Action>,
{concurrentId, signal, withRetries}: AxiosOptions = {},
) {
const base64 = !settingsManager.readUserSettingsValue(
SETTING_KEYS.BINARY_DATA_IN_PLAIN_TEXT_DISPLAY,
true,
);
const base64 = params.base64;

return this.post<QueryAPIResponse<Action> | ErrorResponse | null>(
this.getPath('/viewer/json/query'),
Expand Down
10 changes: 7 additions & 3 deletions src/store/reducers/query/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ interface SendQueryParams extends QueryRequestParams {
// flag whether to send new tracing header or not
// default: not send
enableTracingLevel?: boolean;
base64?: boolean;
}

// Stream query receives queryId from session chunk.
Expand All @@ -239,7 +240,7 @@ export const queryApi = api.injectEndpoints({
endpoints: (build) => ({
useStreamQuery: build.mutation<null, StreamQueryParams>({
queryFn: async (
{query, database, querySettings = {}, enableTracingLevel},
{query, database, querySettings = {}, enableTracingLevel, base64},
{signal, dispatch, getState},
) => {
const startTime = Date.now();
Expand Down Expand Up @@ -294,6 +295,7 @@ export const queryApi = api.injectEndpoints({
: undefined,
output_chunk_max_size: DEFAULT_STREAM_CHUNK_SIZE,
concurrent_results: DEFAULT_CONCURRENT_RESULTS || undefined,
base64,
},
{
signal,
Expand Down Expand Up @@ -343,7 +345,7 @@ export const queryApi = api.injectEndpoints({
}
},
}),
useSendQuery: build.mutation<null, SendQueryParams>({
useSendQuery: build.mutation({
queryFn: async (
{
actionType = 'execute',
Expand All @@ -352,7 +354,8 @@ export const queryApi = api.injectEndpoints({
querySettings = {},
enableTracingLevel,
queryId,
},
base64,
}: SendQueryParams,
{signal, dispatch, getState},
) => {
const startTime = Date.now();
Expand Down Expand Up @@ -396,6 +399,7 @@ export const queryApi = api.injectEndpoints({
? Number(querySettings.timeout) * 1000
: undefined,
query_id: queryId,
base64,
},
{signal},
);
Expand Down
1 change: 1 addition & 0 deletions src/types/api/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ export interface SendQueryParams<Action extends Actions> {
query_id?: string;
limit_rows?: number;
internal_call?: boolean;
base64?: boolean;
}

export interface StreamQueryParams<Action extends Actions> extends SendQueryParams<Action> {
Expand Down
Loading