Skip to content

Commit

Permalink
CDPD-68882: [editor] Replace the editor_autocomplete_timeout setting …
Browse files Browse the repository at this point in the history
…with disable_source_autocomplete

Setting editor_autocomplete_timeout is currently dead code, so I decided to deprecate it with this commit and replace it with a boolean disable_source_autocomplete setting. The timeout setting was introduced in the past to support two use cases. First the old autocomplete selection panel would not show until all autocomplete suggestions had been fetched, to speed things up in the UI this could be set to a few seconds, today this is no longer needed as results are populated as they come in from the backend. The second use case was to disable API-based autocomplete all together by setting it 0, this has been broken for at least 3 years now.

Given that the timeout is no longer necessary I went for a boolean replacement.

(cherry picked from commit 1291cc8)
(cherry picked from commit 8658c860e7bd4bbf508d68e0c945389ae88e7ddf)
Change-Id: Idd0093dc26eb764cc46c8cb6b8ee6d965e247f36
(cherry picked from commit 63339c14db70a17b80966f303c93c076c5f3ff3b)
  • Loading branch information
JohanAhlen authored and Athithyaa Selvam committed Apr 17, 2024
1 parent 912bb7e commit aae0861
Show file tree
Hide file tree
Showing 25 changed files with 721 additions and 402 deletions.
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const normalGlobals = [];

const hueGlobals = [
// global_js_constants.mako
'AUTOCOMPLETE_TIMEOUT',
'CACHEABLE_TTL',
'CSRF_TOKEN',
'DOCUMENT_TYPES',
Expand Down
1 change: 0 additions & 1 deletion apps/beeswax/src/beeswax/templates/execute.mako
Original file line number Diff line number Diff line change
Expand Up @@ -1173,7 +1173,6 @@ var autocompleter = new AceAutocompleteWrapper({
user: HIVE_AUTOCOMPLETE_USER,
oldEditor: true,
optEnabled: false,
timeout: AUTOCOMPLETE_TIMEOUT
});
var truncateOutput = function (obj) {
Expand Down
8 changes: 5 additions & 3 deletions desktop/conf.dist/hue.ini
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,11 @@ http_500_debug_mode=false
# Choose whether to allow multi tenancy or not.
## enable_organizations=false

# Editor autocomplete timeout (ms) when fetching columns, fields, tables etc.
# To disable this type of autocompletion set the value to 0.
## editor_autocomplete_timeout=30000
# Choose whether the editor autocomplete should gather suggestions from external source or not. The editor
# autocomplete uses various sources for its suggestions, listing databases, tables, columns files etc. The results are
# cached on the client (see cacheable_ttl) so the calls are kept to a minimum but if you prefer to disable these calls
# all together from the editor set this to true.
## disable_source_autocomplete=false

# Enable saved default configurations for Hive, Impala, Spark, and Oozie.
## use_default_configuration=false
Expand Down
8 changes: 5 additions & 3 deletions desktop/conf/pseudo-distributed.ini.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,11 @@
# Choose whether to allow multi tenancy or not.
## enable_organizations=false

# Editor autocomplete timeout (ms) when fetching columns, fields, tables etc.
# To disable this type of autocompletion set the value to 0.
## editor_autocomplete_timeout=30000
# Choose whether the editor autocomplete should gather suggestions from external source or not. The editor
# autocomplete uses various sources for its suggestions, listing databases, tables, columns files etc. The results are
# cached on the client (see cacheable_ttl) so the calls are kept to a minimum but if you prefer to disable these calls
# all together from the editor set this to true.
## disable_source_autocomplete=false

# Enable saved default configurations for Hive, Impala, Spark, and Oozie.
## use_default_configuration=false
Expand Down
11 changes: 5 additions & 6 deletions desktop/core/src/desktop/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -1926,11 +1926,11 @@ def get_instrumentation_default():
help=_('Choose whether to enable SQL syntax check or not.')
)

EDITOR_AUTOCOMPLETE_TIMEOUT = Config(
key='editor_autocomplete_timeout',
type=int,
default=30000,
help=_('Timeout value in ms for autocomplete of columns, tables, values etc. 0 = disabled.')
DISABLE_SOURCE_AUTOCOMPLETE = Config(
key='disable_source_autocomplete',
type=coerce_bool,
default=False,
help=_('Choose whether the editor autocomplete should gather suggestions from external sources or not.')
)

ENABLE_HUE_5 = Config(
Expand Down Expand Up @@ -2057,7 +2057,6 @@ def is_chunked_fileuploader_enabled():
),
))


def task_server_default_result_directory():
"""Local directory to store task results."""
return 'file://%s' % get_run_root('logs')
Expand Down
1 change: 0 additions & 1 deletion desktop/core/src/desktop/js/apps/editor/EditorViewModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ export default class EditorViewModel {
}
});

this.autocompleteTimeout = options.autocompleteTimeout;
this.lastNotifiedDialect = undefined;

this.combinedContent = ko.observable();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,306 @@
// Licensed to Cloudera, Inc. under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. Cloudera, Inc. licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import * as Vue from 'vue';
import * as CatalogApi from 'catalog/api';
import { CancellablePromise } from 'api/cancellablePromise';
import AutocompleteResults, { Suggestion } from './AutocompleteResults';
import dataCatalog from 'catalog/dataCatalog';
import huePubSub from 'utils/huePubSub';
import I18n from 'utils/i18n';
import * as sqlUdfRepository from 'sql/reference/sqlUdfRepository';
import sqlReferenceRepository from 'sql/reference/sqlReferenceRepository';
import sleep from 'utils/timing/sleep';
import * as hueConfig from 'config/hueConfig';
import { Ace } from 'ext/ace';
import Executor from '../../../execution/executor';
import sqlAnalyzerRepository from 'catalog/analyzer/sqlAnalyzerRepository';
import { AutocompleteParseResult } from 'parse/types';
import { SetDetails, UdfDetails } from 'sql/reference/types';
import { HueConfig } from 'config/types';

describe('AutocompleteResults.ts', () => {
jest.spyOn(Vue, 'onBeforeUnmount').mockImplementation(() => undefined);

const sourceMetaSpy = jest
.spyOn(CatalogApi, 'fetchSourceMetadata')
.mockImplementation(options => {
if (options.entry.path.length === 0) {
return CancellablePromise.resolve(JSON.parse('{"status": 0, "databases": ["default"]}'));
}
if (options.entry.path.length === 1) {
return CancellablePromise.resolve(
JSON.parse(
'{"status": 0, "tables_meta": [{"comment": "comment", "type": "Table", "name": "foo"}, {"comment": null, "type": "View", "name": "bar_view"}, {"comment": null, "type": "Table", "name": "bar"}]}'
)
);
}
if (options.entry.path.length === 2) {
return CancellablePromise.resolve(
JSON.parse(
'{"status": 0, "support_updates": false, "hdfs_link": "/filebrowser/view=/user/hive/warehouse/customers", "extended_columns": [{"comment": "", "type": "int", "name": "id"}, {"comment": "", "type": "string", "name": "name"}, {"comment": "", "type": "struct<email_format:string,frequency:string,categories:struct<promos:boolean,surveys:boolean>>", "name": "email_preferences"}, {"comment": "", "type": "map<string,struct<street_1:string,street_2:string,city:string,state:string,zip_code:string>>", "name": "addresses"}, {"comment": "", "type": "array<struct<order_id:string,order_date:string,items:array<struct<product_id:int,sku:string,name:string,price:double,qty:int>>>>", "name": "orders"}], "columns": ["id", "name", "email_preferences", "addresses", "orders"], "partition_keys": []}'
)
);
}
if (options.entry.path.length === 3) {
return CancellablePromise.resolve(
JSON.parse(
'{"status": 0, "comment": "", "type": "struct", "name": "email_preferences", "fields": [{"type": "string", "name": "email_format"}, {"type": "string", "name": "frequency"}, {"fields": [{"type": "boolean", "name": "promos"}, {"type": "boolean", "name": "surveys"}], "type": "struct", "name": "categories"}]}'
)
);
}
if (options.entry.path.length > 3) {
return CancellablePromise.resolve(
JSON.parse(
'{"status": 0, "fields": [{"type": "boolean", "name": "promos"}, {"type": "boolean", "name": "surveys"}], "type": "struct", "name": "categories"}'
)
);
}
return CancellablePromise.reject();
});

const createSubject = (): AutocompleteResults => {
const mockEditor = () => ({
getTextBeforeCursor: () => 'foo',
getTextAfterCursor: () => 'bar'
});

const mockExecutor = {
connector: () => ({ id: 'hive', dialect: 'hive' }),
database: () => 'default',
namespace: () => ({ id: 'defaultNamespace' }),
compute: () => ({ id: 'defaultCompute' })
} as Executor;

return new AutocompleteResults({
temporaryOnly: false,
executor: mockExecutor,
sqlAnalyzerProvider: sqlAnalyzerRepository,
sqlReferenceProvider: sqlReferenceRepository,
editor: mockEditor as unknown as Ace.Editor
});
};

beforeEach(() => {
huePubSub.publish('assist.clear.all.caches');
dataCatalog.disableCache();
});

afterEach(() => {
dataCatalog.enableCache();
jest.resetAllMocks();
});

it('should handle parse results with keywords', async () => {
const subject = createSubject();
const suggestions: Suggestion[] = [];

await subject.update(
{
lowerCase: true,
suggestKeywords: [
{ value: 'BAR', weight: 1 },
{ value: 'FOO', weight: 2 }
]
} as AutocompleteParseResult,
suggestions
);

expect(suggestions.length).toBe(2);
expect(suggestions[0].meta).toBe(I18n('keyword'));
expect(suggestions[0].value).toBe('bar');
expect(suggestions[0].weightAdjust).toBe(1);
expect(suggestions[1].meta).toBe(I18n('keyword'));
expect(suggestions[1].value).toBe('foo');
expect(suggestions[1].weightAdjust).toBe(2);
});

it('should handle parse results with identifiers', async () => {
const subject = createSubject();
const suggestions: Suggestion[] = [];

await subject.update(
{
lowerCase: false,
suggestIdentifiers: [
{ name: 'foo', type: 'alias' },
{ name: 'bar', type: 'table' }
]
} as AutocompleteParseResult,
suggestions
);

expect(suggestions.length).toBe(2);
expect(suggestions[1].meta).toBe('table');
expect(suggestions[1].value).toBe('bar');
expect(suggestions[0].meta).toBe('alias');
expect(suggestions[0].value).toBe('foo');
});

it('should handle parse results with functions', async () => {
const spy = jest
.spyOn(sqlUdfRepository, 'getUdfsWithReturnTypes')
.mockImplementation(async () =>
Promise.resolve([
{
name: 'count',
returnTypes: ['BIGINT'],
arguments: [[{ type: 'T' }]],
signature: 'count(col)',
draggable: 'count()',
description: 'some desc'
}
])
);
const subject = createSubject();
const suggestions: Suggestion[] = [];

await subject.update(
{
lowerCase: false,
suggestFunctions: {}
} as AutocompleteParseResult,
suggestions
);

await sleep(0);

expect(spy).toHaveBeenCalled();

expect(suggestions.length).toEqual(1);
const udfDetails = suggestions[0].details as UdfDetails;
expect(udfDetails.arguments).toBeDefined();
expect(udfDetails.signature).toBeDefined();
expect(udfDetails.description).toBeDefined();
});

it('should handle parse results with udf argument keywords', async () => {
const spy = jest
.spyOn(sqlUdfRepository, 'getArgumentDetailsForUdf')
.mockImplementation(async () => Promise.resolve([{ type: 'T', keywords: ['a', 'b'] }]));
const subject = createSubject();
const suggestions: Suggestion[] = [];

await subject.update(
{
lowerCase: false,
udfArgument: {
name: 'someudf',
position: 1
}
} as AutocompleteParseResult,
suggestions
);

await sleep(0);

expect(spy).toHaveBeenCalled();

expect(suggestions.length).toEqual(2);
expect(suggestions[0].value).toEqual('a');
expect(suggestions[1].value).toEqual('b');
});

it('should handle parse results set options', async () => {
const spy = jest.spyOn(sqlReferenceRepository, 'getSetOptions').mockImplementation(
async dialect =>
new Promise(resolve => {
expect(dialect).toEqual(subject.executor.connector().dialect);
resolve({
OPTION_1: {
description: 'Desc 1',
type: 'Integer',
default: 'Some default'
},
OPTION_2: {
description: 'Desc 2',
type: 'Integer',
default: 'Some default'
}
});
})
);
const subject = createSubject();
const suggestions: Suggestion[] = [];

await subject.update(
{
lowerCase: false,
suggestSetOptions: {}
} as AutocompleteParseResult,
suggestions
);

await sleep(0);

expect(spy).toHaveBeenCalled();

expect(suggestions.length).toEqual(2);
expect((suggestions[0].details as SetDetails).description).toBeDefined();
expect((suggestions[1].details as SetDetails).type).toBeDefined();
});

it('should fetch from source when disable_source_autocomplete is set to false', async () => {
jest.spyOn(hueConfig, 'getLastKnownConfig').mockImplementation(
() =>
({
app_config: {
editor: {
source_autocomplete_disabled: false
}
}
} as HueConfig)
);

const subject = createSubject();
const suggestions: Suggestion[] = [];

await subject.update(
{
lowerCase: false,
suggestDatabases: {}
} as AutocompleteParseResult,
suggestions
);

expect(sourceMetaSpy).toHaveBeenCalled();
});

it('should not fetch from source when disable_source_autocomplete is set to true', async () => {
jest.spyOn(hueConfig, 'getLastKnownConfig').mockImplementation(
() =>
({
app_config: {
editor: {
source_autocomplete_disabled: true
}
}
} as HueConfig)
);
const subject = createSubject();
const suggestions: Suggestion[] = [];

await subject.update(
{
lowerCase: false,
suggestDatabases: {}
} as AutocompleteParseResult,
suggestions
);

expect(sourceMetaSpy).not.toHaveBeenCalled();
});
});
Loading

0 comments on commit aae0861

Please sign in to comment.