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

docs: updating documentation for NIMf connection example in Metal Connection Resource #734

Merged
merged 8 commits into from
Jul 22, 2024

Conversation

srushti-patl
Copy link
Contributor

@srushti-patl srushti-patl commented Jul 18, 2024

-Updated template, doc and schema files for NIMF connection Documentation change:

  • resources/fabric-conenction
  • respurces/metal-connection
  • data-sources/metal-connection
  • Update example file names for metal and fabric connection resource

@srushti-patl srushti-patl requested review from a team as code owners July 18, 2024 21:19
Copy link
Contributor

@ctreatma ctreatma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes need to be made in the templates directory; once the templates are updated, you can run make docs to regenerate the docs from the updated templates.

In the templates, you should consider replacing the hard-coded attribute listing with {{ .SchemaMarkdown | trimspace }} (see #724).

Copy link
Contributor

@thogarty thogarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should update the descriptions in the direct schema of each data_source/resource and then use the schema markdown suggestion to regenerate the attribute list directly from what the schema is now. That way it will be dynamically updated without having to make docs changes manually.

@@ -56,3 +56,4 @@ In addition to all arguments above, the following attributes are exported:
* `link_status` - Port link status.
* `virtual_circuit_ids` - List of IDs of virtual cicruits attached to this port.
* `token` - (Deprecated) Fabric Token required to configure the connection in Equinix Fabric with the [equinix_fabric_connection](../resources/fabric_connection.md) resource or from the [Equinix Fabric Portal](https://fabric.equinix.com/dashboard). If your organization already has connection service tokens enabled, use `service_tokens` instead.
* `authorization_code` - Fabric Authorization Code to configure the NIMF connection in Equinix Fabric with the [equinix_fabric_connection](../resources/fabric_connection.md) resource or from the [Equinix Fabric Portal](https://fabric.equinix.com/dashboard).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of manually updating this I would recommend using {{ .SchemaMarkdown }} to replace the attributes list and have it build the correct attribute from the description in the schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have used {{ .SchemaMarkdown }} to replace attributes list but manually updated attribute reference for metal connection

@@ -57,3 +65,4 @@ In addition to all arguments above, the following attributes are exported:
* `ports` - List of connection ports - primary (`ports[0]`) and secondary (`ports[1]`). Schema of port is described in documentation of the [equinix_metal_connection datasource](../data-sources/metal_connection.md).
* `service_tokens` - List of connection service tokens with attributes required to configure the connection in Equinix Fabric with the [equinix_fabric_connection](./fabric_connection.md) resource or from the [Equinix Fabric Portal](https://fabric.equinix.com/dashboard). Scehma of service_token is described in documentation of the [equinix_metal_connection datasource](../data-sources/metal_connection.md).
* `token` - (Deprecated) Fabric Token required to configure the connection in Equinix Fabric with the [equinix_fabric_connection](./fabric_connection.md) resource or from the [Equinix Fabric Portal](https://fabric.equinix.com/dashboard). If your organization already has connection service tokens enabled, use `service_tokens` instead.
* `authorization_code` - Fabric Authorization code to configure the NIMF connection with Cloud Service Provider through Equinix Fabric with the [equinix_fabric_connection](./fabric_connection.md) resource from the [Equinix Developer Portal](https://developer.equinix.com/dev-docs/fabric/getting-started/fabric-v4-apis/connect-metal-to-amazon-web-services).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of manually updating this I would recommend using {{ .SchemaMarkdown }} to replace the attributes list and have it build the correct attribute from the description in the schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have used {{ .SchemaMarkdown }} to replace attributes list but manually updated attribute reference for metal connection

@@ -149,7 +157,7 @@ Optional:

Optional:

- `authentication_key` (String) Authentication key for provider based connections
- `authentication_key` (String) Authentication key for provider based connections or Metal NIMF connections
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you're in here; instead of manually updating this I would recommend using {{ .SchemaMarkdown }} to replace the attributes list and have it build the correct attribute from the description in the schema.

@@ -30,6 +30,14 @@ Use this resource to request the creation an Interconnection asset to connect wi

{{tffile "examples/resources/metal_connection/example_3.tf"}}

### Shared Connection with authorization_code Non-redundant NIMF connection from Equinix Metal to a Cloud Service Provider via Equinix Fabric Port

{{tffile "examples/resources/metal_connection/example_4.tf"}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the docs migrator doesn't know the purpose of individual example files, but we do; I recommend using more descriptive file names for these example files, such as shared_nimf_to_csp.tf and shared_nimf_from_fcr.tf instead of example_<n>.tf. The only time we should use a generic name for an example file is when there is a single example, called example.tf, since that one will be automatically discovered by the generator (with the caveat that the generator has a weird inconsistency in the conventional paths for examples, mentioned in #724).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have renamed the metal and fabric connection example.

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.88%. Comparing base (f8c2ab9) to head (1fd29eb).
Report is 11 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #734       +/-   ##
===========================================
+ Coverage   34.44%   51.88%   +17.43%     
===========================================
  Files         151      153        +2     
  Lines       20710    20994      +284     
===========================================
+ Hits         7133    10892     +3759     
+ Misses      13415     9672     -3743     
- Partials      162      430      +268     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

thogarty
thogarty previously approved these changes Jul 19, 2024
Copy link
Contributor

@thogarty thogarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @srushti-patl

* `tags` - (Optional) String list of tags.
* `vlans` - (Optional) Only used with shared connection. Vlans to attach. Pass one vlan for Primary/Single connection and two vlans for Redundant connection.
* `service_token_type` - (Optional) Only used with shared connection. Type of service token to use for the connection, a_side or z_side. (**NOTE: To support the legacy non-automated way to create connections, terraform will not check if `service_token_type` is specified. If your organization already has `service_token_type` enabled, be sure to specify it or the connection will return a legacy connection token instead of a service token**)
{{ .SchemaMarkdown | trimspace }}

## Attributes Reference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ## Attributes Reference can also be removed; it's part of the .SchemaMarkdown output

@@ -20,7 +20,7 @@ Use this data source to retrieve a [connection resource](https://metal.equinix.c

The following arguments are supported:

* `connection_id` - (Required) ID of the connection resource.
{{ .SchemaMarkdown | trimspace }}

## Attributes Reference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ## Attributes Reference can also be removed; it's part of the .SchemaMarkdown output


Read-Only:

- `id` (String)
Copy link
Contributor

@ctreatma ctreatma Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These have lost their description because it was hard-coded in the docs. For the metal_connection docs you could either keep the hard-coded attributes for now or update the resource and data source schemas under internal/resources/metal/connection/ to add the missing descriptions for these properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctreatma I have kept hard coded attributes for metal_connections docs.

Copy link
Contributor

@thogarty thogarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@srushti-patl srushti-patl merged commit e2b34f2 into main Jul 22, 2024
8 of 11 checks passed
@srushti-patl srushti-patl deleted the CXF-97293-NIMF-Documentation-Fix branch July 22, 2024 20:19
Copy link

This PR is included in version 2.3.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants