-
Notifications
You must be signed in to change notification settings - Fork 3
Prepare change-from-tfplan for terraform mapping #7
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
Conversation
Since we have messages that regularly break the 32k barrier (see [BACKEND-Z](https://overmindtech.sentry.io/issues/4295740369)) and we already have the api-server gateway client configured to accept messages of up to this size (see SetReadLimit in api-server).
dylanratcliffe
left a comment
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.
Unless I'm mistaken this will re-introduce a previously squashed bug. Also the use of GOTOs sent me on a rollercoaster of emotions from Windows batch file PTSD all the way back to "actually these do make sense"
dylanratcliffe
left a comment
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.
Couple of potential simplifications but looks good
| // fall through from all "final" query states, check if there's still queries in progress; | ||
| // only break from the loop if all queries have already been sent | ||
| // TODO: see above, still needs DefaultStartTimeout implemented to account for slow sources | ||
| allDone := allDone(ctx, activeQueries, lf) |
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 think this is necessary but no hard in having it I guess. If you were to get rid of this it would make sense to also get rid if the whole *sdp.GatewayResponse_QueryStatus branch too
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.
Another notch for having to write a proper client library as the responder status does not provide detailed information on what is and is not done. I'll leave it as is for now as a note-to-self that this part of SDP also needs to be considered as part of the client work.
| // fall through from all "final" query states, check if there's still queries in progress; | ||
| // only break from the loop if all queries have already been sent | ||
| // TODO: see above, still needs DefaultStartTimeout implemented to account for slow sources | ||
| allDone := allDone(ctx, activeQueries, lf) |
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.
Once again I think this could be completely removed if you want, no pressure though
Co-authored-by: Dylan <dylanratcliffe@outlook.com>
This still doesn't do any actual tfplan mapping, but now has all the structure in place to work with the data from the plan and the aws-source's docs-data.
Example run: