-
Notifications
You must be signed in to change notification settings - Fork 228
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
Implement terraform Resource methods on resourceMetricsEndpointScrapeJob #1812
Implement terraform Resource methods on resourceMetricsEndpointScrapeJob #1812
Conversation
In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically. |
4fffc5a
to
43a37f0
Compare
6fde641
to
a70d7b0
Compare
…erraform-resource-methods
@@ -27,9 +27,6 @@ const ( | |||
|
|||
func NewClient(authToken string, rawURL string, client *http.Client) (*Client, error) { | |||
parsedURL, err := url.Parse(rawURL) | |||
if parsedURL.Scheme != "https" { |
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.
Removing this check in order to mock with normal httptest server in acceptance tests.
The failing acceptance tests/cloudinstance seems to be from OnCall tests.. https://github.com/grafana/terraform-provider-grafana/actions/runs/11295196530/job/31417373511?pr=1812 |
@@ -100,14 +101,14 @@ func (r *resourceMetricsEndpointScrapeJob) Schema(ctx context.Context, req resou | |||
}, | |||
}, | |||
"name": schema.StringAttribute{ | |||
Description: "The name of the Metrics Endpoint Scrape Job. Part of the Terraform Resource ID.", | |||
Description: "The name of the metrics endpoint scrape job. Part of the Terraform Resource ID.", | |||
Required: true, | |||
PlanModifiers: []planmodifier.String{ | |||
stringplanmodifier.RequiresReplace(), | |||
}, | |||
}, | |||
"enabled": schema.BoolAttribute{ |
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.
I don't see enabled being used elsewhere, such as in the docs. Is it wired up correctly?
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.
(I think it was wired up correctly, but we did find an issue in the API. We've decided to remove the "enabled" field for now. To be implemented later: https://github.com/grafana/cloud-onboarding/issues/7842)
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.
One question about enabled
- but otherwise looks good!
resp.State.SetAttribute(ctx, path.Root("stack_id"), jobTF.StackID) | ||
resp.State.SetAttribute(ctx, path.Root("name"), jobTF.Name) | ||
resp.State.SetAttribute(ctx, path.Root("authentication_method"), jobTF.AuthenticationMethod) | ||
resp.State.SetAttribute(ctx, path.Root("url"), jobTF.URL) | ||
resp.State.SetAttribute(ctx, path.Root("scrape_interval_seconds"), jobTF.ScrapeIntervalSeconds) |
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.
Matt, since we talked about this:
I tested out the scenario where we set the entire object: As you pointed out before, this unintentionally detects drift because we're trying to set an empty username/password.
So we do indeed want to set only the non-sensitive attributes.
{ | ||
category: "Connections", | ||
testCheck: func(t *testing.T, filename string) { | ||
t.Skip() // TODO: Make all examples work |
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.
This is copied from the other implementations: this is re-running the acceptance tests here in TestAccExamples, but without the declaration of the mock httptest server, these tests fail. I'm not sure what the long-term solution plan is.
The acceptance tests still run and are passing elsewhere.
name = "scrape-job-name" | ||
authentication_method = "basic" | ||
authentication_basic_username = "my-username" | ||
authentication_basic_password = "my-password" |
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.
Can these examples include scrape interval seconds?
Searched |
This implements the methods for a terraform Resource on resourceMetricsEndpointScrapeJob.
Contributes to https://github.com/grafana/cloud-onboarding/issues/7670 and https://github.com/grafana/cloud-onboarding/issues/7673