wmedvede commented on code in PR #311: URL: https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/311#discussion_r1425116853
########## controllers/platform/services/properties.go: ########## @@ -0,0 +1,131 @@ +// Copyright 2023 Apache Software Foundation (ASF) +// +// Licensed 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. + +package services + +import ( + "fmt" + "net/url" + "strings" + + operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" + "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/constants" + + "github.com/magiconair/properties" +) + +func generateReactiveURL(postgresSpec *operatorapi.PersistencePostgreSql, schema string, namespace string, dbName string, port int) (string, error) { + if len(postgresSpec.JdbcUrl) > 0 { + s := strings.TrimLeft(postgresSpec.JdbcUrl, "jdbc:") + u, err := url.Parse(s) + if err != nil { + return "", err + } + ret := fmt.Sprintf("%s://", u.Scheme) + if len(u.User.Username()) > 0 { + p, ok := u.User.Password() + if ok { + ret = fmt.Sprintf("%s%s:%s@", ret, u.User.Username(), p) + } + } + ret = fmt.Sprintf("%s%s%s?", ret, u.Host, u.Path) + kv, err := url.ParseQuery(u.RawQuery) + if err != nil { + return "", err + } + spv := schema Review Comment: I believe that in this case we are analyzing the JdbcUrl that was provided by the user, and thus, we should infer the schema from it, but not default to svp := schema, not good in my opinion. why? if the JdbcUrl has no schema, flyway will create the tables in the database default schema, since the tables are created by using the JdbcUrl non reactive datasource. And, then, the reactive datasource will point to a different schema. See that it remains consistent to what we have in: configurePostgreSqlEnv if len(postgresql.JdbcUrl) > 0 { dataSourceUrl = postgresql.JdbcUrl //here we take the jdbc url as is. } ########## controllers/platform/services/properties.go: ########## @@ -0,0 +1,131 @@ +// Copyright 2023 Apache Software Foundation (ASF) +// +// Licensed 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. + +package services + +import ( + "fmt" + "net/url" + "strings" + + operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" + "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/constants" + + "github.com/magiconair/properties" +) + +func generateReactiveURL(postgresSpec *operatorapi.PersistencePostgreSql, schema string, namespace string, dbName string, port int) (string, error) { + if len(postgresSpec.JdbcUrl) > 0 { + s := strings.TrimLeft(postgresSpec.JdbcUrl, "jdbc:") + u, err := url.Parse(s) + if err != nil { + return "", err + } + ret := fmt.Sprintf("%s://", u.Scheme) + if len(u.User.Username()) > 0 { + p, ok := u.User.Password() + if ok { + ret = fmt.Sprintf("%s%s:%s@", ret, u.User.Username(), p) + } + } + ret = fmt.Sprintf("%s%s%s?", ret, u.Host, u.Path) + kv, err := url.ParseQuery(u.RawQuery) + if err != nil { + return "", err + } + spv := schema + if v, ok := kv["search_path"]; ok { + for _, val := range v { + if len(val) != 0 { + spv = v[0] + } + } + } else if v, ok := kv["currentSchema"]; ok { + for _, val := range v { + if len(val) != 0 { + spv = v[0] Review Comment: here we try to infer the schema, good! ########## controllers/platform/services/properties.go: ########## @@ -0,0 +1,131 @@ +// Copyright 2023 Apache Software Foundation (ASF) +// +// Licensed 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. + +package services + +import ( + "fmt" + "net/url" + "strings" + + operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" + "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/constants" + + "github.com/magiconair/properties" +) + +func generateReactiveURL(postgresSpec *operatorapi.PersistencePostgreSql, schema string, namespace string, dbName string, port int) (string, error) { + if len(postgresSpec.JdbcUrl) > 0 { + s := strings.TrimLeft(postgresSpec.JdbcUrl, "jdbc:") + u, err := url.Parse(s) + if err != nil { + return "", err + } + ret := fmt.Sprintf("%s://", u.Scheme) + if len(u.User.Username()) > 0 { + p, ok := u.User.Password() + if ok { + ret = fmt.Sprintf("%s%s:%s@", ret, u.User.Username(), p) + } + } + ret = fmt.Sprintf("%s%s%s?", ret, u.Host, u.Path) + kv, err := url.ParseQuery(u.RawQuery) + if err != nil { + return "", err + } + spv := schema + if v, ok := kv["search_path"]; ok { + for _, val := range v { + if len(val) != 0 { + spv = v[0] Review Comment: here we try to infer the schema, good! ########## controllers/platform/services/properties.go: ########## @@ -0,0 +1,131 @@ +// Copyright 2023 Apache Software Foundation (ASF) +// +// Licensed 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. + +package services + +import ( + "fmt" + "net/url" + "strings" + + operatorapi "github.com/apache/incubator-kie-kogito-serverless-operator/api/v1alpha08" + "github.com/apache/incubator-kie-kogito-serverless-operator/controllers/profiles/common/constants" + + "github.com/magiconair/properties" +) + +func generateReactiveURL(postgresSpec *operatorapi.PersistencePostgreSql, schema string, namespace string, dbName string, port int) (string, error) { + if len(postgresSpec.JdbcUrl) > 0 { + s := strings.TrimLeft(postgresSpec.JdbcUrl, "jdbc:") + u, err := url.Parse(s) + if err != nil { + return "", err + } + ret := fmt.Sprintf("%s://", u.Scheme) + if len(u.User.Username()) > 0 { + p, ok := u.User.Password() + if ok { + ret = fmt.Sprintf("%s%s:%s@", ret, u.User.Username(), p) + } + } + ret = fmt.Sprintf("%s%s%s?", ret, u.Host, u.Path) + kv, err := url.ParseQuery(u.RawQuery) + if err != nil { + return "", err + } + spv := schema + if v, ok := kv["search_path"]; ok { + for _, val := range v { + if len(val) != 0 { + spv = v[0] + } + } + } else if v, ok := kv["currentSchema"]; ok { + for _, val := range v { + if len(val) != 0 { + spv = v[0] + } + } + } + return fmt.Sprintf("%ssearch_path=%s", ret, spv), nil Review Comment: when we reach this point, if we couldn't infer the schema from the JdbcUrl, we don't set it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
