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.

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

Add paused attribute to cloud_scheduler_job resource #430

Closed
wants to merge 1 commit into from
Closed

Add paused attribute to cloud_scheduler_job resource #430

wants to merge 1 commit into from

Conversation

wyardley
Copy link

Description

Add state and paused attributes to the cloud scheduler job resources

Based on:
GoogleCloudPlatform/magic-modules#6304

Issues Resolved

List any existing issues this PR resolves, or any Discourse or StackOverflow discussion that's relevant.

Please ensure commits have been signed-off for the Developer Certificate of Origin. See https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

Add `state` and `paused` attributes to the cloud scheduler job resources
Based on:
GoogleCloudPlatform/magic-modules#6304

Signed-off-by: William Yardley <[email protected]>
@wyardley wyardley requested a review from a team as a code owner August 19, 2022 04:54
@wyardley
Copy link
Author

@sa-progress @clintoncwolfe I see you all have your own fork of Magic Modules now. I made this PR with the original one; I removed the updates to the headers and stuff to keep this as minimal as possible (and to keep things like having input instead of attribute), but let me know if you want; I can easily add the MMV1 in the headers bit.

Side note: you may want to update the project README and the headers about code generation to make it clear to what extent this project is or isn't using Magic Modules now, and what the process is for requesting upstream changes (or requesting updates to resources), how inspec / Chef's fork is or isn't going to get updated from the GCP magic modules repo, etc.

@wyardley
Copy link
Author

@sa-progress would you be able to take a look at this one?

@samiranand1990
Copy link
Contributor

@sa-progress would you be able to take a look at this one?

@wyardley

Going through your changes will provide input if required.
Thanks

Copy link
Contributor

@samiranand1990 samiranand1990 left a comment

Choose a reason for hiding this comment

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

Please take a look at provided comment.

@@ -52,6 +54,8 @@ def parse
@description = @fetched['description']
@schedule = @fetched['schedule']
@time_zone = @fetched['timeZone']
@state = @fetched['state']
@paused = @fetched['paused']
Copy link
Contributor

Choose a reason for hiding this comment

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

here paused is an enum value under state.
so when you will try to access @fetched['paused'] it will fail.

Please see the response of the api below:

{
"name": string,
"description": string,
"schedule": string,
"timeZone": string,
"userUpdateTime": string,
"state": enum ([State](https://cloud.google.com/scheduler/docs/reference/rest/v1beta1/projects.locations.jobs#State)),
"status": {
object (Status)
},
"scheduleTime": string,
"lastAttemptTime": string,
"retryConfig": {
object (RetryConfig)
},
"attemptDeadline": string,
"legacyAppEngineCron": boolean,

// Union field target can be only one of the following:
"pubsubTarget": {
object (PubsubTarget)
},
"appEngineHttpTarget": {
object (AppEngineHttpTarget)
},
"httpTarget": {
object (HttpTarget)
}
// End of list of possible types for union field target.
}
api -
response api

state enum has below values:
Enum values

Copy link
Author

Choose a reason for hiding this comment

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

Got it. So if I pull out the stuff related to paused completely, this should work because we'll be able to test if state is equal to PAUSED (in the case of a paused job), right?

Copy link
Author

Choose a reason for hiding this comment

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

Or, since paused is a top level attribute in Terraform itself, should the interface here stay the same, but we should just populate it from @fetched['state']?

Copy link
Author

Choose a reason for hiding this comment

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

If this is something we should implement by making an adjustment in inspec's fork of magic-modules, happy to work on trying to implement it that way too (I didn't know it existed at the time I created this).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes! That is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes You can contribute to Magic module that will be the right thing to do.

Copy link
Author

@wyardley wyardley Dec 7, 2022

Choose a reason for hiding this comment

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

@sa-progress I think we basically want
GoogleCloudPlatform/magic-modules#6304

I have cherry-picked that in with attribution to the original author (@SarahFrench) as inspec/magic-modules#6

Because the original commit is also under a license that should be compatible, I've signed off on the commit, which I think is all Ok / allowed?

@wyardley
Copy link
Author

wyardley commented Feb 2, 2023

Closing for inspec/magic-modules#6
@sa-progress can you take a look please tho?

@wyardley wyardley closed this Feb 2, 2023
@wyardley wyardley deleted the wyardley/cloud_scheduler_job_paused branch February 2, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants