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

Fix depiction of defaults and organise them #1754

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Amogh-Bharadwaj
Copy link
Contributor

@Amogh-Bharadwaj Amogh-Bharadwaj commented May 24, 2024

Screenshot 2024-05-25 at 12 36 59 AM

Fixes issues where some params in overview page would show '0' but they would actually have a default value

Organise default values of :
Snapshot number of tables in parallel
Snapshot number of workers
Snapshot number of rows per partition
Pull batch Size
Sync interval

to constant files in Go and UI

Show these defaults when not set in UI (happens when mirror is created via query layer)
Also show publication name in UI

Functionally tested with creating a mirror from UI and Query Layer

@@ -11,7 +11,7 @@ import (

"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgxpool"
"github.com/yuin/gopher-lua"
lua "github.com/yuin/gopher-lua"
Copy link
Member

Choose a reason for hiding this comment

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

you should fix your config to stop doing this on save

@@ -0,0 +1,15 @@
// These are the backend defaults if the user creates a new mirror
// via SQL layer without specifying these settings.
const DefaultSyncInterval = 60;
Copy link
Member

@serprex serprex May 24, 2024

Choose a reason for hiding this comment

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

this seems like it'll be hard to keep in sync. Ideally we'd have build step read values out of go files

vercel/next.js#17479

Copy link
Member

Choose a reason for hiding this comment

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

hack: make defaults protobuf enum

@serprex
Copy link
Member

serprex commented May 27, 2024

not sure if addressed here, but important to differentiate between explicit value matching default & default. For if default changes. Rendering default as placeholder could work

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.

None yet

2 participants