Deprecate grafana field and add log info warning #1192
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rizwana777
force-pushed
the
issue-2049
branch
5 times, most recently
from
0aee209
to
7d826a0
Compare
jgwest
requested changes
Jan 30, 2024
There was a problem hiding this comment.
- See proposed tweak to the deprecation warning, in all the places where it is used.
- Actually, come to think of it, it couldn't hurt to put this message in a
stringconstant.
- Actually, come to think of it, it couldn't hurt to put this message in a
grafana.goincontrollers/argocdcan also be deleted, AFAICT.- You can also search the code for 'grafana', as I think there are some unused constants that can be removed here, too.
|
@Rizwana777: to fix the failing PR checks:
|
Rizwana777
force-pushed
the
issue-2049
branch
5 times, most recently
from
5b39bde
to
468c553
Compare
jgwest
requested changes
Feb 5, 2024
|
@jgwest removed most of the files and contents related to grafana, PTAL |
jgwest
requested changes
Feb 6, 2024
Rizwana777
force-pushed
the
issue-2049
branch
from
5d9a104
to
2a9bcb4
Compare
Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
Rizwana777
force-pushed
the
issue-2049
branch
from
2a9bcb4
to
aa7641d
Compare
Rizwana777
force-pushed
the
issue-2049
branch
from
aa7641d
to
fdc27ec
Compare
jaideepr97
reviewed
Feb 13, 2024
Comment on lines
-312
to
-317
| // newConfigMapWithName creates a new ConfigMap with the given suffix appended to the name. | ||
| // The name for the CongifMap is based on the name of the given ArgCD. | ||
| func newConfigMapWithSuffix(suffix string, cr *argoproj.ArgoCD) *corev1.ConfigMap { | ||
| return newConfigMapWithName(fmt.Sprintf("%s-%s", cr.ObjectMeta.Name, suffix), cr) | ||
| } | ||
|
|
There was a problem hiding this comment.
@Rizwana777 did you mean to get rid of this function? it is used for other configmaps as well
There was a problem hiding this comment.
@jaideepr97 newConfigMapWithSuffix is used only used by grafana right now and I didn't see any place in the code where it is used, so I have removed it , I can add it back if we are using it in future somewhere?
There was a problem hiding this comment.
oops, you're right
my bad
jaideepr97
reviewed
Feb 13, 2024
controllers/argocd/util.go
Outdated
| @@ -669,7 +641,7 @@ func (r *ReconcileArgoCD) redisShouldUseTLS(cr *argoproj.ArgoCD) bool { | |||
| tlsSecretName := types.NamespacedName{Namespace: cr.Namespace, Name: common.ArgoCDRedisServerTLSSecretName} | |||
| err := r.Client.Get(context.TODO(), tlsSecretName, &tlsSecretObj) | |||
| if err != nil { | |||
| if !apierrors.IsNotFound(err) { | |||
There was a problem hiding this comment.
I would prefer we keep this as apierrors as it is more descriptive and allows us to separately import the go errors pkg as well, if needed
|
Thanks for raising this @Rizwana777 , I am personally happy to see grafana getting deprecated from the operator :) |
Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
Rizwana777
force-pushed
the
issue-2049
branch
from
fdc27ec
to
4d8cabe
Compare
jaideepr97
approved these changes
Feb 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
What does this PR do / why we need it:
JIRA Link - https://issues.redhat.com/browse/GITOPS-2049