Return charts for specific namespace and global namespace. #1588
Conversation
cmd/assetsvc/postgresql_utils.go
Outdated
| clauses = append(clauses, "repo_namespace = $1") | ||
| queryParams = append(queryParams, namespace) | ||
| queryParams = append(queryParams, namespace, m.GetKubeappsNamespace()) | ||
| clauses = append(clauses, fmt.Sprintf("(repo_namespace = $%d OR repo_namespace = $%d)", len(queryParams)-1, len(queryParams))) |
There was a problem hiding this comment.
this would be more readable if it were $1 and $2 but I understand why you are calculating those anyway
There was a problem hiding this comment.
Yeah, like I had it initially. Unfortunate given they may not be the first and second... ah, that's what you mean, that if these are included they will always be the first and second params, it's only really the following one which needs to be calculated... true. Updated :)
| @@ -215,3 +217,7 @@ SELECT ID FROM %s WHERE namespace=$1 AND name=$2 | |||
| func (m *PostgresAssetManager) GetDB() PostgresDB { | |||
| return m.DB | |||
| } | |||
|
|
|||
| func (m *PostgresAssetManager) GetKubeappsNamespace() string { | |||
| return m.kubeappsNamespace | |||
There was a problem hiding this comment.
In theory nor the GetDB or this GetKubeappsNamespace are needed if we expose those properties but I am fine anyway. Is there a reason why you added this apart than for standardization?
There was a problem hiding this comment.
Yeah, because when the initial postgres integration was done, the PostgresAssetManagerIface interface was defined above and embedded in the postgresAssetManager struct at https://github.com/kubeapps/kubeapps/blob/master/cmd/assetsvc/postgresql_utils.go#L34 . Since you can't add properties to an interface, only methods, this means that when handling an instance of postgresAssetManager, you can only get to the methods defined on the interface, not the properties of the underlying struct implementing that interface, without adding type conversions etc.
So yes, I do think we could clean these up and remove a layer or two - there seem to be multiple private implementations of both postgres and mongo (for assetsvc and asset-sync) which I don't think are necessary. But I've not attempted to do that work here.
Ref #1521 . Gives the assetsvc knowledge of the kubeapps namespace and updates
getPaginatedChartListso that it returns charts in a specific namespace together with those from the kubeapps (global) namespace.While there, found, tested and fixed another issue related to the SQL params (need to use dynamic numbering for the params).
The asset-sync commands are updated to store the kubeappsNamespace but this is not yet set in the environment or used, though it may enable us to stop the frontend from needing to know the kubeapps namespace.