Skip to content

Conversation

@yoshiokatsuneo
Copy link
Contributor

@yoshiokatsuneo yoshiokatsuneo commented Feb 10, 2025

What type of PR is this?

  • Feature

Description

Before this PR, autocomplete is disabled for tokens more than 5000. That is too small for us, and probably the most of dataware house.

And, anyway, each user can disable autocomplete if they want.
I think it should be up to user to decide whether to use autocomplete or not, and it is better to make hard-coded limit large enough to most user.

This PR changes the limit of the token from 5000 to 1000000. There is no change on memory consumption as the tokens are anyway stored on memory even if autocomplete is disabled.

How is this tested?

  • Manually

I confirmed that there is no problem for token more than 700000, and there is no CPU/memory exhausting.

@denisov-vlad
Copy link
Member

Great! Did the same for my Redash instance.

@eradman
Copy link
Collaborator

eradman commented Feb 12, 2025

This PR changes the limit of the token from 5000 to 1000000. There is no change on memory consumption as the tokens are anyway stored on memory even if autocomplete is disabled.

This is a good point! Should this check be removed altogether?

@yoshiokatsuneo
Copy link
Contributor Author

This PR changes the limit of the token from 5000 to 1000000. There is no change on memory consumption as the tokens are anyway stored on memory even if autocomplete is disabled.

This is a good point! Should this check be removed altogether?

@eradman

Sounds like a plan !
I just made another PR that remove the check altogether.

#7326

@eradman
Copy link
Collaborator

eradman commented Feb 13, 2025

Closing in favor of #7326

@eradman eradman closed this Feb 13, 2025
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.

3 participants