Simplify Service import logic, and allow selecting a version while importing #448
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the service resource importer was doing a full state refresh. This was redundant because the Read function gets called right after the Import one, so the state was being refreshed twice. It also introduced extra logic in the Read function to determine whether it was being called via the Import function or not, to enable the version selection logic to be tweaked. This commit changes the behaviour so that the Import function only has one responsibility; to set cloned_version. The rest of the state read is then fully delegated to the Read function, which needs no extra logic to determine the version, as it should already be set. The Import function has been given some extra logic, to optionally parse a version number from the provided ID, by using an '@' symbol after the ID. If it isn't specified, i.e. only the ID is used, then the Read function will default to the active version, or the latest version if that is not set either.
Integralist
reviewed
Jul 23, 2021
Integralist
approved these changes
Jul 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the Service resource import would do a full state refresh twice: once in the
resourceImportfunction, and again when Terraform callsresourceReadagain afterwards. This PR simplifies this, to only optionally set thecloned_versionin theresourceImportfunction, and delegate the rest of the state refresh to theresourceReadcalled by Terraform automatically.If no version is specified for the import, then the Read function will detect
cloned_versionis 0, and select either the active version or latest version. This logic hasn't changed, but theisImportflag argument is no longer needed.In simplifying the import logic, it seemed like a good opportunity to add a new feature: allowing
@and a version number to be appended to the ID whilst importing, to enable the user to choose a version. This is optional, and the previous method of only specifying the ID without the version is still supported.