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

Fix fetching of app_engine_standard_app_version data #6388

Conversation

pmarschik
Copy link

When omitting the view query parameter some fields (such as handlers) are not returned and lead terraform to show changes in the resource when there are none.

@ghost ghost added the size/xs label May 15, 2020
@ghost ghost requested review from slevenick and megan07 and removed request for slevenick May 15, 2020 12:09
@megan07
Copy link
Contributor

megan07 commented May 18, 2020

Hi @pmarschik! Thanks for pointing this out! Unfortunately, this file is autogenerated and needs to be updated in the magic-modules repo. I started looking into it a bit, and it seems to be a little more involved that just adding view=FULL, but you can take a stab at it here: https://github.com/GoogleCloudPlatform/magic-modules/blob/master/products/appengine/api.yaml#L228
From what I saw updating the self_link to include the view=FULL at the end would be the first step, but I think you may also have to add a bit in appengine/terraform.yaml under StandardAppVersion like this

      handlers: !ruby/object:Overrides::Terraform::PropertyOverride
        default_from_api: true

This will write the default handlers to state.
I didn't get to fully test it or see if anymore needed to be done, because I realized I was spending a little too much time on it, but if you'd rather one of us make the updates, please feel free to open a new bug and reference this PR and someone will take a look at it. Feel free to assign it to me if you'd like and I will get to it when I get the chance.
If you're feeling really ambitious, it looks like this also affects the FlexibleAppVersion resource as well, and what I noticed is that handlers isn't an option in there (but should be, since there is a default). If you choose to fix this yourself, it would be awesome if you could make the change in both places, but again, if that's too much, we're happy to make the changes as well.
Thanks!

@pmarschik
Copy link
Author

@megan07 Thanks for getting back! I've opened an issue #6417 and started a PR GoogleCloudPlatform/magic-modules#3536 which solves the concrete problem I faced when using the terraform-provider-google. I'm sorry but I couldn't invest too much time in catching the other edge cases you mentioned. Feel free to either use my PR as a base or start from scratch (I still need to get my employer to sign the CLA)

@ghost
Copy link

ghost commented Jun 18, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Jun 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants