Add separate a module for the api package. #1856
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
boris-smidt-klarrio
requested review from
NissesSenap,
weisdd,
ishanjainn,
theSuess,
hubeadmin and
pb82
as code owners
api/go.mod
Outdated
Comment on lines
8
to
10
| github.com/onsi/ginkgo/v2 v2.22.2 | ||
| github.com/onsi/gomega v1.36.2 | ||
| github.com/openshift/api v0.0.0-20250207102212-9e59a77ed2e0 |
There was a problem hiding this comment.
I suspect that these dependencies add a lot of extra packages to the module.
So there are still many extra dependencies included.
The reduction is from 92 packages to 67 packages.
There was a problem hiding this comment.
Does this fix the original issue of version incompatibility though?
The only reason we include the openshift api here is to allow route overrides here: https://github.com/grafana/grafana-operator/blob/master/api/v1beta1/typeoverrides.go#L331
So if you really want to get rid of the openshift dependency here, we can pull up the imported fields
There was a problem hiding this comment.
I've vendored the openshift api now.
In the release notes it should be noted that to remove the openapi package we had to make a breaking change to the imported api.
But not in the kubernetes api itself.
boris-smidt-klarrio
changed the title
boris-smidt-klarrio
changed the title
theSuess
approved these changes
Feb 17, 2025
|
The linter has a small correction for the typeoverrides imports as well. You'll also need to run |
github-actions
bot
added
the
documentation
|
@boris-smidt-klarrio @theSuess since this PR was merged, I'm having issues w/ my local environment. I can't seem to get the modules in a good state. i have tried any ideas how to resolve this? update: i think it is due to my feature branch adding a dependency on controllers/metrics from w/in the API module :( - i will have to move this. resolved: addressed in 8d0c441 |
diurnalist
added a commit
to diurnalist/grafana-operator
that referenced
this pull request
Feb 19, 2025
since grafana#1856, the api module is separated. this poses problems with the approach to have the CRs expose which metrics correspond to their types via the GrafanaContentMetrics() function, because we create an import loop. recognizing that we're measuring essentially the same thing b/w library panels and dashboards, this updates the metrics to indicate this more explicitly. the following changes are made to metrics: `grafana_operator_dashboard_requests`: * renamed to `grafana_operator_content_requests`, but will always have a label `kind="GrafanaDashboard"`. * renamed `dashboard` label to `resource` `grafana_operator_dashboard_revision_requests`: * renamed to `grafana_operator_revision_requests`, but will always have a label `kind="GrafanaDashboard"`. * renamed `dashboard` label to `resource`
diurnalist
added a commit
to diurnalist/grafana-operator
that referenced
this pull request
Feb 19, 2025
since grafana#1856, the api module is separated. this poses problems with the approach to have the CRs expose which metrics correspond to their types via the GrafanaContentMetrics() function, because we create an import loop. recognizing that we're measuring essentially the same thing b/w library panels and dashboards, this updates the metrics to indicate this more explicitly. the following changes are made to metrics: `grafana_operator_dashboard_requests`: * renamed to `grafana_operator_content_requests`, but will always have a label `kind="GrafanaDashboard"`. * renamed `dashboard` label to `resource` `grafana_operator_dashboard_revision_requests`: * renamed to `grafana_operator_revision_requests`, but will always have a label `kind="GrafanaDashboard"`. * renamed `dashboard` label to `resource`
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 27, 2025
* add grafanalibrarypanel CRD types this adds a new GrafanaLibraryPanel CRD, updates all the auto-generated assets derived from the CRD type, and updates existing RBAC and Helm definitions to reference the new type. Co-authored-by: Matt Delaney <matt.delaney@getcruise.com> * track library panels in grafana CR this registers the new CRD with the Grafana CRD so that we can track which library panels have been provisioned with the operator, similar to how other resources are tracked. Co-authored-by: Matt Delaney <matt.delaney@getcruise.com> * add reconciler implementation Co-authored-by: Matt Delaney <matt.delaney@getcruise.com> * disallow fetching library panels from grafana.com much of the content resolving/fetching logic is shared b/w dashboards and library panels, but there is one awkward thing where library panels are not currently (to my knowledge) available via grafana.com. rather than extract just that fetch logic out and only apply it to dashboards, i've opted to simply disable them as a source. an error will be issued if a user tries to configure this field. it is not the most graceful way to handle this, as it's not communicated via the CR validation, but given the effort required to bend the code to do this "right", and given the likelihood of somebody trying to configure a library panel via a grafana.com url (which does not exist right now), we are choosing the easy button. * remove dashboard name from generic functions * add chainsaw tests * mark proposal as decided * enable separate metrics for content resources to enable dashboards and library panels to track their fetch operations separately, split up the metrics by resource. this is accomplished by adding a new interface method to the GrafanaContentResource, and the returned struct holds references to the relevant metrics, if any. this also adds an additional guard to the http round tripper so that if no metric is defined, we don't panic. * remove lazy folder creation and simplify logic * move metrics out of api module since #1856, the api module is separated. this poses problems with the approach to have the CRs expose which metrics correspond to their types via the GrafanaContentMetrics() function, because we create an import loop. recognizing that we're measuring essentially the same thing b/w library panels and dashboards, this updates the metrics to indicate this more explicitly. the following changes are made to metrics: `grafana_operator_dashboard_requests`: * renamed to `grafana_operator_content_requests`, but will always have a label `kind="GrafanaDashboard"`. * renamed `dashboard` label to `resource` `grafana_operator_dashboard_revision_requests`: * renamed to `grafana_operator_revision_requests`, but will always have a label `kind="GrafanaDashboard"`. * renamed `dashboard` label to `resource` * add backwards-compatibility for metrics and simplify reconcile to make the dashboard url request metric backwards compatible, this updates the signature of the roundtripper to be a bit more general, so we can pass a function that is called at the end of the response rather than explicitly increment a metric. this allows us to increment multiple metrics in the case of the dashboard url request, one for the old and one for the new. * refactor: pass metrics to round_tripper directly * refactor: remove intermediate structs to further simplify logic * register invalidspec condition on validation errs * Update api/v1beta1/grafanalibrarypanel_types.go Co-authored-by: Steffen Baarsgaard <steff.bpoulsen@gmail.com> * regen --------- Co-authored-by: Matt Delaney <matt.delaney@getcruise.com> Co-authored-by: Dominik Süß <dominik@suess.wtf> Co-authored-by: Steffen Baarsgaard <steff.bpoulsen@gmail.com>
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.
No description provided.