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

feat: add max row limit #3546

Merged
merged 6 commits into from
Oct 24, 2022
Merged

feat: add max row limit #3546

merged 6 commits into from
Oct 24, 2022

Conversation

ZeRego
Copy link
Contributor

@ZeRego ZeRego commented Oct 21, 2022

Closes: #3498

Description:

New env LIGHTDASH_QUERY_MAX_LIMIT that defaults to 5000

max row limit

Error for saved queries that exceed the limit.

Screenshot 2022-10-24 at 10 00 40

@ZeRego ZeRego self-assigned this Oct 21, 2022
@netlify
Copy link

netlify bot commented Oct 21, 2022

Deploy Preview for peaceful-bassi-cbf284 canceled.

Name Link
🔨 Latest commit c0b9db7
🔍 Latest deploy log https://app.netlify.com/sites/peaceful-bassi-cbf284/deploys/6356901423ff5e0008531f95

@owlas owlas requested a deployment to feat/add-row-limit - jaffle_db PR #3546 October 21, 2022 17:25 — with Render Abandoned
Copy link
Contributor

@TuringLovesDeathMetal TuringLovesDeathMetal left a comment

Choose a reason for hiding this comment

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

nice one! looks good 😚 👌

Copy link
Collaborator

@rephus rephus left a comment

Choose a reason for hiding this comment

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

Code looks good. Also great job with everything that is not code related: adding env variable, asking the community..

onLimitChange,
}) => {
const { health } = useApp();
const methods = useForm<{ limit: string }>({
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is the limit a string and not a number ?

methods={methods}
onSubmit={handleSubmit}
>
<Input
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not using a numeric input ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a really annoying behaviour when editing a number in the numeric input.
Eg: limit is 500 but you want 600. So the user deletes the 5 to then type 6. But the input instantly converts the 00 to 0 to be a valid number.

@@ -45,3 +45,8 @@ export const isValidEmailDomain: FieldValidator<string[]> =
: undefined;
}
};

export const isOnlyNumbers: FieldValidator<string> = (fieldName) => (value) =>
!value || value.match(/\D/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@IrakliJani IrakliJani left a comment

Choose a reason for hiding this comment

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

good stuff

Comment on lines 24 to 26
useEffect(() => {
setValue('limit', limit?.toString());
}, [limit, setValue]);
Copy link
Contributor

Choose a reason for hiding this comment

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

form has defaultValues option that you can use instead


const handleSubmit = useCallback(
(data: { limit: string }) => {
console.log('handleSubmit');
Copy link
Contributor

Choose a reason for hiding this comment

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

log

@owlas owlas temporarily deployed to feat/add-row-limit - lightdash PR #3546 October 24, 2022 13:16 — with Render Destroyed
@ZeRego ZeRego merged commit 012d702 into main Oct 24, 2022
@ZeRego ZeRego deleted the feat/add-row-limit branch October 24, 2022 13:16
github-actions bot pushed a commit that referenced this pull request Oct 24, 2022
# [0.297.0](0.296.0...0.297.0) (2022-10-24)

### Features

* add max row limit ([#3546](#3546)) ([012d702](012d702))
* support multiple pivots in table visualisations  ([#3537](#3537)) ([eecc880](eecc880))
@github-actions
Copy link

🎉 This PR is included in version 0.297.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when requesting a large amount of rows
5 participants