-
Notifications
You must be signed in to change notification settings - Fork 138
constants: centralize GLOBAL state support list and use helper in catalog delta (Fixes #601) #610
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,5 +40,17 @@ const ( | |
| MySQL DriverType = "mysql" | ||
| Oracle DriverType = "oracle" | ||
| ) | ||
| // GlobalStateSupportedDrivers is the canonical list of connectors that support GLOBAL state | ||
| var GlobalStateSupportedDrivers = []DriverType{Postgres, MySQL} | ||
|
|
||
| // IsGlobalStateSupported reports whether the given connector type supports GLOBAL state | ||
| func IsGlobalStateSupported(connectorType string) bool { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please remove this function from constants.go. constants.go is static page which should have no computation based methods. |
||
| dt := DriverType(connectorType) | ||
| for _, d := range GlobalStateSupportedDrivers { | ||
| if d == dt { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
| var RelationalDrivers = []DriverType{Postgres, MySQL, Oracle} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,7 +208,8 @@ func GetStreamsDelta(oldStreams, newStreams *Catalog, connectorType string) *Cat | |
|
|
||
| // flag for connector which have global state support | ||
| // TODO: create an array of global state supported connectors in constants | ||
| globalStateSupportedConnector := connectorType == string(constants.Postgres) || connectorType == string(constants.MySQL) | ||
| // flag for connectors which have global state support (centralized in constants) | ||
| globalStateSupportedConnector := constants.IsGlobalStateSupported(connectorType) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here you are calling a loop to check the bool. but previously it was a simple OR. this is bit overkill I believe. please do change accordingly. try to use OR itself. |
||
|
|
||
| for namespace, newMetadatas := range newStreams.SelectedStreams { | ||
| for _, newMetadata := range newMetadatas { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the PR must be made from staging, not master. Please read the contribution docs at olake.io/docs