Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for CRUD of holidays to client #5

Merged
merged 1 commit into from
Jan 5, 2023
Merged

Add support for CRUD of holidays to client #5

merged 1 commit into from
Jan 5, 2023

Conversation

sd2k
Copy link
Contributor

@sd2k sd2k commented Jan 4, 2023

This PR adds support for the new Holidays feature of
Grafana ML to the client. The client supports CRUD
of holidays as well as linking jobs to holidays using
the LinkJobHolidays method.

@sd2k sd2k requested a review from csmarchbanks January 4, 2023 10:32
sd2k added a commit to grafana/terraform-provider-grafana that referenced this pull request Jan 4, 2023
@sd2k
Add support for Machine Learning holidays
586c77a 
Note that holidays can be created and added to jobs, but not vice
versa, despite the API supporting it. This is because the API
would return an updated value for the holiday's `jobs` field if we
exposed it via Terraform, and we have no way of updating that from
the job resource's CreateContext (as far as I'm aware).

NOTE: this currently depends on an unreleased version of the
machine-learning-go-client repo; once
grafana/machine-learning-go-client#5 is merged,
I'll adjust this PR and mark it ready for review.
@sd2k sd2k mentioned this pull request Jan 4, 2023
Merged
sd2k added a commit to grafana/terraform-provider-grafana that referenced this pull request Jan 4, 2023
@sd2k
Add support for Machine Learning holidays
ecd659a 
Note that holidays can be created and added to jobs, but not vice
versa, despite the API supporting it. This is because the API
would return an updated value for the holiday's `jobs` field if we
exposed it via Terraform, and we have no way of updating that from
the job resource's CreateContext (as far as I'm aware).

NOTE: this currently depends on an unreleased version of the
machine-learning-go-client repo; once
grafana/machine-learning-go-client#5 is merged,
I'll adjust this PR and mark it ready for review.
sd2k added a commit to grafana/terraform-provider-grafana that referenced this pull request Jan 4, 2023
@sd2k
Add support for Machine Learning holidays
214a4d3 
Note that holidays can be created and added to jobs, but not vice
versa, despite the API supporting it. This is because the API
would return an updated value for the holiday's `jobs` field if we
exposed it via Terraform, and we have no way of updating that from
the job resource's CreateContext (as far as I'm aware).

NOTE: this currently depends on an unreleased version of the
machine-learning-go-client repo; once
grafana/machine-learning-go-client#5 is merged,
I'll adjust this PR and mark it ready for review.
sd2k added a commit to grafana/terraform-provider-grafana that referenced this pull request Jan 4, 2023
@sd2k
Add support for Machine Learning holidays
8b81194 
Note that holidays can be created and added to jobs, but not vice
versa, despite the API supporting it. This is because the API
would return an updated value for the holiday's `jobs` field if we
exposed it via Terraform, and we have no way of updating that from
the job resource's CreateContext (as far as I'm aware).

NOTE: this currently depends on an unreleased version of the
machine-learning-go-client repo; once
grafana/machine-learning-go-client#5 is merged,
I'll adjust this PR and mark it ready for review.
csmarchbanks
csmarchbanks reviewed Jan 4, 2023
View reviewed changes
mlapi/holiday.go Outdated Show resolved Hide resolved
csmarchbanks
csmarchbanks reviewed Jan 4, 2023
View reviewed changes
Copy link
Collaborator

@csmarchbanks csmarchbanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small thing then 👍

mlapi/holiday_test.go Outdated Show resolved Hide resolved
csmarchbanks
csmarchbanks reviewed Jan 4, 2023
View reviewed changes
mlapi/job.go Outdated Show resolved Hide resolved
@sd2k
Add support for CRUD of holidays to client
8dcd5f4 
This PR adds support for the new Holidays feature of
Grafana ML to the client. The client supports CRUD
of holidays as well as linking jobs to holidays using
the LinkJobHolidays method.
@sd2k sd2k merged commit 89f9f50 into main Jan 5, 2023
@sd2k sd2k deleted the add-holiday-support branch January 5, 2023 17:29
sd2k added a commit to grafana/terraform-provider-grafana that referenced this pull request Jan 5, 2023
@sd2k
Add support for Machine Learning holidays
eccfbb3 
Note that holidays can be created and added to jobs, but not vice
versa, despite the API supporting it. This is because the API
would return an updated value for the holiday's `jobs` field if we
exposed it via Terraform, and we have no way of updating that from
the job resource's CreateContext (as far as I'm aware).

NOTE: this currently depends on an unreleased version of the
machine-learning-go-client repo; once
grafana/machine-learning-go-client#5 is merged,
I'll adjust this PR and mark it ready for review.
julienduchesne pushed a commit to grafana/terraform-provider-grafana that referenced this pull request Jan 10, 2023
@sd2k @julienduchesne
Add support for Machine Learning holidays
1a9cab7 
Note that holidays can be created and added to jobs, but not vice
versa, despite the API supporting it. This is because the API
would return an updated value for the holiday's `jobs` field if we
exposed it via Terraform, and we have no way of updating that from
the job resource's CreateContext (as far as I'm aware).

NOTE: this currently depends on an unreleased version of the
machine-learning-go-client repo; once
grafana/machine-learning-go-client#5 is merged,
I'll adjust this PR and mark it ready for review.
julienduchesne pushed a commit to grafana/terraform-provider-grafana that referenced this pull request Jan 10, 2023
@sd2k
Add support for Machine Learning holidays (#764)
58edc9e 
* Add support for Machine Learning holidays

Note that holidays can be created and added to jobs, but not vice
versa, despite the API supporting it. This is because the API
would return an updated value for the holiday's `jobs` field if we
exposed it via Terraform, and we have no way of updating that from
the job resource's CreateContext (as far as I'm aware).

NOTE: this currently depends on an unreleased version of the
machine-learning-go-client repo; once
grafana/machine-learning-go-client#5 is merged,
I'll adjust this PR and mark it ready for review.

* Fix docs for machine learning holiday resource

* Handle 404s when reading ML jobs/holidays
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csmarchbanks csmarchbanks csmarchbanks approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sd2k @csmarchbanks