chore: Defines new schema scaffolding command using terraform code generation tools #1827
Conversation
AgustinBettati
changed the title
AgustinBettati
force-pushed
the
CLOUDP-221360
branch
from
32eb4c5 to
ac3c8c6
Compare
AgustinBettati
force-pushed
the
CLOUDP-221360
branch
from
ac3c8c6 to
48f9c0f
Compare
|
Will look into failing synk checks that appeared in this PR. Currently not able to obtain more details. |
| @@ -12,6 +12,8 @@ Thanks for your interest in contributing to MongoDB Atlas Terraform Provider, th | |||
| - [Running Acceptance Tests](#running-acceptance-tests) | |||
There was a problem hiding this comment.
not for this PR but I'm feeling more and more the pain @gssbzn mentions about contributing file size :-)
maybe at some moment we can have in this file the very basic stuff that people need to start contributing, and refer to other files for more specific details.
There was a problem hiding this comment.
yes makes sense, one option I see is to define a README.md in scaffolding folder and reference it from CONTRIBUTING.md file.
|
|
||
|
|
||
| # URL to download Atlas Admin API Spec | ||
| atlas_admin_api_spec="https://raw.githubusercontent.com/mongodb/atlas-sdk-go/main/openapi/atlas-api-transformed.yaml" |
There was a problem hiding this comment.
Could we explain somewhere why we are using this file instead of https://mongodb-mms-prod-build-server.s3.amazonaws.com/openapi/d05d140a757e46f9ce25ee52b17ae91335003c32-v2-2023-11-15.json? I want to make sure to capture the reasons behind this decision so that we don't forget it
There was a problem hiding this comment.
I added details in the Spec document with the reasoning behind this decision, as well as a point in the risks section. If we are for seeing a significant maintenance cost we can adjust here.
GNUmakefile
Outdated
| scaffold-schemas: | ||
| @tools/scaffold/scripts/schema-scaffold.sh $(name) |
There was a problem hiding this comment.
should the scaffold command call the scaffold-schemas?
There was a problem hiding this comment.
It is not the initial idea, but can be looked into. The scaffold-schemas requires the generator_config.yml file to be defined with the mapping of the endpoints that will be used. CLOUDP-221362: Adjust existing scaffolding command will include the generation of a template for the generator_config.yml file, but will require defining the endpoints and possibly additional configuration before running scaffold-schemas.
GNUmakefile
Outdated
| # details on usage can be found in CONTRIBUTING.md under "Scaffolding Schema and Model Definitions" | ||
| .PHONY: scaffold-schemas | ||
| scaffold-schemas: | ||
| @tools/scaffold/scripts/schema-scaffold.sh $(name) |
There was a problem hiding this comment.
| @tools/scaffold/scripts/schema-scaffold.sh $(name) | |
| @tools/scaffold/scripts/schema-scaffold.sh $(resource_name) |
In my PR for generating doc #1829 I am using resource_name to add additional context. let me know if you have a strong opinion in using name instead
There was a problem hiding this comment.
resource_name sounds good. This is a small nit: both scaffold commands receive the resource_name in camel case, while in docs receives it is in snake case. I think the formats received in both cases are out of convenience, just stating in case it is something simple to align.
There was a problem hiding this comment.
unfortunately, we are forced to use the snake case for tfplugindocs. We could rename the name in the script but it would require additional code to handle it
Co-authored-by: Andrea Angiolillo <[email protected]>
… alignment with docs command
| @@ -115,9 +115,14 @@ link-git-hooks: ## Install git hooks | |||
| update-atlas-sdk: ## Update the atlas-sdk dependency | |||
| ./scripts/update-sdk.sh | |||
|
|
|||
| # details on usage can be found in CONTRIBUTING.md under "Creating New Resource and Data Sources" | |||
| # details on usage can be found in CONTRIBUTING.md under "Scaffolding initial Code and File Structure" | |||
There was a problem hiding this comment.
Are there any reasons why we are not adding how to use the script here instead of asking the user to open another file? I'd expect to have here a description on how to use the script and then add in CONTRIBUTING how to use the makefile command
There was a problem hiding this comment.
Contributing file does contain more detailed description of the intent, limitations, and further guidance. I can add an example of how command is run in the make file for users which are familiar with this information. Is this question related to how the docs command will be documented? If it is does not have much context to be provided, I believe it makes sense to just describe how it is run in the make file.
Description
Link to any related issue(s): CLOUDP-221360
This PR also includes updates to our CONTRIBUTING.md guide providing guidance on the usage of the command.
Type of change:
Required Checklist:
Further comments