chore: Implement full wait functionality for long-running operations #3305

lantoli · 2025-04-28T18:46:33Z

Description

Implement full wait functionality for long-running operations.

A DELETED state is used for 404 http code or empty JSON when waiting for Delete operations.
Body request is extracted to be directly a param in UntypedAPICall to make its use cleaner

Link to any related issue(s): CLOUDP-315494

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

EspenAlbert · 2025-04-29T14:12:22Z

internal/common/autogen/handle_operations.go

-	d.AddError(fmt.Sprintf("Error %s in %s", errSummary, opName), err.Error())
+// waitForChanges waits until a long-running operation is done.
+// It returns the latest JSON response from the API so it can be used to update the response state.
+func waitForChanges(ctx context.Context, wait *WaitReq, client *config.MongoDBClient) ([]byte, error) {


~~Will this work for delete? Maybe for delete there will be no response body?~~
Nice. see the notFound and emptyJSON below now. Awesome 👏

EspenAlbert · 2025-04-29T14:13:52Z

internal/common/autogen/handle_operations.go

+// waitForChanges waits until a long-running operation is done.
+// It returns the latest JSON response from the API so it can be used to update the response state.
+func waitForChanges(ctx context.Context, wait *WaitReq, client *config.MongoDBClient) ([]byte, error) {
+	if len(wait.TargetStates) == 0 {


Does this deserve a warning? When are 0 TargetStates a valid configuration?

I had some doubts, but I think it deserves an error as it's a misconfiguration problem, changed here: 1dd2089

e.g. error:

EspenAlbert · 2025-04-29T14:17:48Z

internal/service/searchdeploymentapi/resource.go

-			TargetStates:      []string{},
-			TimeoutSeconds:    600,
+			TargetStates:      []string{"DELETED"},
+			TimeoutSeconds:    10800,


Are we missing a call to: resp.State.RemoveResource(ctx) in this method?

according to HC doc, RemoveResource should only be called from Read. Wait is only for Create/Update/Delete

EspenAlbert

Very nice! I can see this being used in curated resources too!
Just need a wrapper function that decodes the body response to the proper SDK class (can be generic)

AgustinBettati

LGTM

AgustinBettati · 2025-04-29T16:08:47Z

internal/common/autogen/handle_operations.go

+	bodyResp, err := waitForChanges(ctx, wait, client)
+	if err != nil || isEmptyJSON(bodyResp) {
 		return err
 	}
-	if respBody == nil {
+	if err := Unmarshal(bodyResp, model); err != nil {


nit: Is there a need for this isEmptyJSON check? from refresh function implementation I understand empty json implies DELETED, so an error would be returned

in the general cases it's not needed, but it's a precaution just in case there are some edge cases, e.g. some resource (for some reason) defines DELETED target in Create or Update.

* chore: Defining helper function for making untyped API calls (#3215) * adopting adjusted branch and doing successful GET * remove project example calls * formatting * linting * include struct for passing parameters * inline api call * chore: Define template for resource operations file (#3227) * create directories if not present when generating new resource * initial template * small adjustments to resource template * include package and resource name * add generated resource * fix golden files * leave comment clarifying meaning of file permission parameter * adjust permission of generated files * chore: Define conversion function for setting API response values into TF Model (non-nested) (#3230) * Create Unmarshal and basic test * fix linter * unmarshal strings * bools * numbers * fix linter * error tests * doc * fix linter * unsupported tests * typo * chore: Support code generation for making POST/GET/DELETE API calls (#3242) * adjusting code generation logic with hardcoded values * refactor case sensitive info * passing unit tests * allow version header not being defined in config and obtained from API Spec * define unit test for resource file generation * fix ordering * adjust template * renaming functions * extract version header to a variable * chore: Define conversion function for generating API request body from Resource Model in create (non-nested) (#3239) * create Marshal * marshal string * make marshal and umarshal func naming more consistent * TestMarshalUnsupported and TestMarshalPanic * panic messages * failing test for omitjson * typo * implement omitjson * createonly * adjust comment * simplify const * rename creatonly tag to omitjsonupdate * document Marshal * fix tfsdk tags * chore: Adjust model generation to include tags relevant for conversion logic of marshalling (#3244) * initial adjustment of model and golden file adjustment * handle generation * support parsing of req body usage * updating pushbasedlogexport * adding comment in merge attributes * small doc adjustment * renaming createonly to omitjsonupdate * add omit all to timeouts attribute * define var group * chore: Adjust code generation to call conversion functions (#3249) * read * read resource * fix Read * Create * update golden file * typo message * chore: Fix string case in resulting json of Marshal call (#3251) * fix string case for json marshalling * configure pble * remove from provider * chore: Configure database_user_api autogeneration (#3254) * detect if generate resource is not up-to-date * custom_db_role_api config * custom_db_role_api files * database_user_api * comment custom_db_role * add generate-resource-check depedency * chore: Support processing PATCH request properties and inferring OmitUpdateBody (#3256) * TDD :) adding failing test * implementing logic to parse request properties in PATCH request * adjust test resource API spec * fix case of using parent computability when merging nested attributes * fix case where post and patch request have different computability * avoid processing properties from request is they are read only * clean up of comments * moving order * refactor to existingAttr, found := * refactor addOrUpdate into smaller function, also simplified logic in mergeNestedAttributes * small comment adjustment * chore: Support list nested attributes in conversion logic for generating API request from Resource Model (#3255) * marshal list of strings * types.Object * renames * types.Set * types.Map * doc * refactor getAttr logic * fix JSON key names * TestMarshalNestedMultiLevel * chore: Add code generation for update operation (#3258) * adjust template * adjust test golden file * adding configuration for http method * chore: Move generate resource Github action to its own file (#3264) * extract generate-resource-check * TEMPORARY: check GHA * Revert "TEMPORARY: check GHA" This reverts commit b1d43df. * add api resources * TEMPORARY change api file * Revert "TEMPORARY change api file" This reverts commit b29dabe. * TEMPORARY change not in api * Revert "TEMPORARY change not in api" This reverts commit 14cc52c. * Reapply "TEMPORARY: check GHA" This reverts commit cc1a392. * Revert "Reapply "TEMPORARY: check GHA"" This reverts commit 92e6a3a. * chore: Support import operation in generated resources (#3265) * add import function with unit testing * add import function with unit testing * add import function with unit testing * adjust code generation to include call to generic import function * add more specific error message * renaming req body usage enums * remove linter exception * renaming of construct to build * close response body after reading * small renaming * extract common error string * rename template variable to ImportIDAttributes * adding seperate error message of read of body fails * chore: Function `UntypedAPICall` return errors when API responds with status code > 300 (#3267) * Return errors when API responds with status code > 300 * remove registering resources * add todo comment on go mod * chore: Remove generation of ObjType models in resource generation (#3274) * add limitation comment * avoid generating obj types in generation * change order * order alphabetically * chore: Support nested attributes in conversion logic for generating Resource Model from API response (#3261) * failing TestUnmarshalNestedAllTypes * keepKeyCase doc * marshal bool * rename jsonResp * simple case test pass for case map[string]any * rename model struct * skip attributes that are not in the model * test case: model attr types in objects must match JSON types * pass test case: model attr types in objects must match JSON types * add float and bool to modelTest * unkown and null attributes in model to get unmarshalled into * compare model instead of individual attrs * support null object as model source * convertUnknownToNull * refactor getObjAttrsAndTypes and getNullAttr * setObjAttrModel * test case: JSON maps not supported yet * more test cases in TestUnmarshalErrors * extract tom setAttrModel and setObjAttrTfModel * getNullAttr for objects * Unmarshal recursive support for objects * refactor setObjAttrModel * support list and set of strings at root level * failing test for object list * getTfAttr * recursive logic in getCollectionElements * fix use of newValue * set example * small doc change * Map comment * split file for marshal and unmarshal * don't convert unknown to null in this PR * recursive lists and sets * chore: Rename autogeneration to autogen (#3278) * rename package * rename autogen struct tag * chore: Convert unknown values to null values (#3281) * fill unkowns in first-level * rename to PrepareResponseModel * recursive objects * recursive tests * fix golden files * move zero-len collection logic to Unmarshal * reuse elements in collections * rename to ResolveUnknowns * update doc * chore: Add CI acc tests for custom_db_role_api and database_user_api (#3279) * acc test config * basic database_user_api test * install tools for goimports * TestAutogenResourcesNotEnabled * fix attributes * custom db role basic test * remove test * comment TestAutogenResourcesNotEnabled * fix database_user_api test * fix customdb_role_api test * chore: Simplify Unmarshal (#3287) * remove duplicated Unmarshal logic * add autogen folder to trigger acc tests * show attribute name in errors * improve error messages * chore: Add CI acc tests for mongodbatlas_push_based_log_export_api and mongodbatlas_search_deployment_api (#3291) * add mongodbatlas_push_based_log_export_api basic test to CI * generate search_deployment_api * add mongodbatlas_search_deployment_api basic test to CI * aws vars in CI acc tests * fix tests bc not running ops * wait in Delete * add reference to CLOUDP-314960 * change search_deployment version so delete works with 404 error * increate wait time for search_deployment delete * chore: Adjust make command to support generating documentation for autogenerated resources (#3297) * adjust docs generation script to support generated autogenerated docs * adjust provide test to support _api_v* fromat as well * add search deployment docs * adjust comment * add comment clarification * adjusting naming * move comments within phony definition * add comment about limitation * include enable-autogen in generate resources workflow * chore: Create template model for wait functionality (#3296) * hardcode values in the template model directly * move values to operations * search_deployment state attr * add return after errors * adjust timeouts * update golden test files * sleep in WaitForChanges * pass params to WaitForChangesReq * also return http.Response so it can be used to update the response state * doc WaitForChanges return params * HandleCreate * rename TestAccCustomDBRoleAPI_basic * HandleRead * HandleUpdate * rename GenericImportOperation to HandleImport * HandleDelete * remove unneeded consts * request structs for operation handlers * pass Wait info to Create and Update * remove test limitations * handleWait * callAPIWithBody and callAPIWithoutBody * don't consider delete wait errors * adjust config.yml * improve error messages * chore: Allow configuring alias for path parameters when names do not match properties in req/resp bodies (#3300) * adjust config and fix for aliases in path params * adjust test * add project and resource policy generation * small change to cluster config * formatting * add new resource in make file * adding generated docs * clarify limitation * avoid provider file causing false positive when finding differences in generated code and docs of generated resources * chore: Support nested attributes with same name avoiding generation of duplicate model names (#3303) * support multiple nested modles with same parent property * formatting * new code generation * add docs * change parentName to ancestorsName * map (#3306) * chore: Implement full wait functionality for long-running operations (#3305) * wip * update templates * state attribute with JSON naming * improve not found logic * delete TODO comment * revert timeout * typo * error if no pending states * chore: Adding CI tests for working project and resource policy generated resources (#3307) * adding test for project api * add resource policy tests * add project and resource policy in PR acceptance test runner for autogen group * config fixes * adjust template to use state for path param values in patch * removal of tags is not yet supported * adjust golden files * use state instead of plan for wait call params in update * add todo in comment * chore: Define long lasting operation information in configuration file (#3313) * rename state attribute to state property * move hardcoded wait values over to config file * adding DELETE in config * adding comment * clean up * adding comment on state property * supporting map type model with null value (#3321) * use untyped client funcs * fix linter error: QF1003: could use tagged switch on attr.ReqBodyUsage * run: make generate-resources * run: make tools enable-autogen generate-resources * move autogen resources to internal/serviceapi * TEMPORARY check catching api resources * Revert "TEMPORARY check catching api resources" This reverts commit 4f6a0d0. * remove autogen resource docs * don't check autogen resource docs * TEMPORARILY: change generated code * Revert "TEMPORARILY: change generated code" This reverts commit ce13e84. * adjust doc * no need to register autogen resources to check if they're generated * removed outdate comment * feedback about autogen resources without acc tests --------- Co-authored-by: Agustin Bettati <bettatiagustin@gmail.com>

lantoli added 6 commits April 28, 2025 16:43

wip

0f2e027

update templates

a1ecd5f

state attribute with JSON naming

8b42fb8

improve not found logic

c55710f

delete TODO comment

25d9fe1

revert timeout

8967657

lantoli marked this pull request as ready for review April 29, 2025 10:02

lantoli requested a review from a team as a code owner April 29, 2025 10:02

typo

69d5988

EspenAlbert reviewed Apr 29, 2025

View reviewed changes

EspenAlbert approved these changes Apr 29, 2025

View reviewed changes

error if no pending states

1dd2089

AgustinBettati approved these changes Apr 29, 2025

View reviewed changes

lantoli merged commit 077ff87 into CLOUDP-301808-poc-autogeneration Apr 29, 2025
42 checks passed

lantoli deleted the CLOUDP-315494_wait_full branch April 29, 2025 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: Implement full wait functionality for long-running operations #3305

chore: Implement full wait functionality for long-running operations #3305

Uh oh!

lantoli commented Apr 28, 2025 •

edited

Loading

Uh oh!

EspenAlbert Apr 29, 2025 •

edited

Loading

Uh oh!

EspenAlbert Apr 29, 2025

Uh oh!

lantoli Apr 29, 2025

Uh oh!

EspenAlbert Apr 29, 2025

Uh oh!

lantoli Apr 29, 2025

Uh oh!

EspenAlbert left a comment

Uh oh!

AgustinBettati left a comment

Uh oh!

AgustinBettati Apr 29, 2025

Uh oh!

lantoli Apr 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chore: Implement full wait functionality for long-running operations #3305

chore: Implement full wait functionality for long-running operations #3305

Uh oh!

Conversation

lantoli commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change:

Required Checklist:

Further comments

Uh oh!

EspenAlbert Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EspenAlbert Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

lantoli Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

EspenAlbert Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

lantoli Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

EspenAlbert left a comment

Choose a reason for hiding this comment

Uh oh!

AgustinBettati left a comment

Choose a reason for hiding this comment

Uh oh!

AgustinBettati Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

lantoli Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lantoli commented Apr 28, 2025 •

edited

Loading

EspenAlbert Apr 29, 2025 •

edited

Loading

lantoli Apr 29, 2025 •

edited

Loading