Add field&resource descriptions to CRD manifests from scraped metadata #64
Conversation
| @@ -242,8 +242,10 @@ func (r *Resource) scrapeFieldDocs(doc *html.Node, fieldXPath string) { | |||
| if r.ArgumentDocs == nil { | |||
| r.ArgumentDocs = make(map[string]string) | |||
| } | |||
| if r.ArgumentDocs[attrName] != "" && r.ArgumentDocs[attrName] != strings.TrimSpace(docStr) { | |||
There was a problem hiding this comment.
I added this statement to not override the set value for the related field. Of course reason of this choice based on an observation and assumption.
For conflicted fields in schemas, I observed that the first ones can be more inclusive or explanatory. I did not check a lot of resource but I want to give an example:
When we go to the latest description in the registry (for repeated fields), we converges to observation field description or less explanatory one.
So, the best solution resolving the issue in the following comment: https://github.com/upbound/official-providers/issues/297#issuecomment-1188997453
But for now, in the scraper tool, (as far as I understand) there is not a hierarchal relationship between fields&subfields. I will also share some statistics in a different comment in this PR. So, my humble request from honorable reviewers would be to see the other comment.
As a last word, this is open for discussion. Maybe @ulucinar can has a good point for reverting this.
| @@ -156,4 +156,5 @@ func (f *Field) AddToResource(g *Builder, r *resource, typeNames *TypeNames) { | |||
| } | |||
|
|
|||
| g.comments.AddFieldComment(typeNames.ParameterTypeName, f.FieldNameCamel, f.Comment.Build()) | |||
| g.comments.AddFieldComment(typeNames.ObservationTypeName, f.FieldNameCamel, f.Comment.Build()) | |||
There was a problem hiding this comment.
I added this statement to pass descriptions also to observation fields. What are your thoughts?
|
Statistics about conflicted fields: Number of Total Scraped Description: 16311 The value in the last row is more valuable than the second. Because even if the keys of the field conflict, if the values are the same, this is not actually a problem. The problem arises if the values are not same. In addition, it would not be entirely correct to conclude that the total number of affected fields is 1080. Because in some schemes a field repeats more than two times. So, in the worst case, our wrong documented field rate is This will be very significant progress and after this PR we will have |
Signed-off-by: Sergen Yalçın <[email protected]>
There was a problem hiding this comment.
As you pointed out, the scraped metadata is not in the correct form, i.e. tree structure, for us to do look-ups using fieldpaths. My first inclination would be to fix that first and then use it instead of relying on uniqueness of the field name unless the case is rare. And with the numbers you're showing, it's not rare but definitely not worth to block this feature. 94% is simply irresistible.
So, let's move forward and open an issue to build the tree structure ASAP. Because if there is no correct representation at this lowest level, then we can't really build more features on top of it reliably and we don't know the number for other TF providers.
@sergenyalcin Good job on extracting the statistics!
Signed-off-by: Sergen Yalçın <[email protected]>
|
https://marketplace.upbound.io/providers/upbound/sergen-demo/v0.0.1 This is a demo repository that contains the field descriptions. |
|
https://github.com/upbound/official-providers/issues/536 follow-up issue about the tree structure for schema fields in scraped metadata |
| @@ -31,12 +31,12 @@ resources: | |||
| ] | |||
| } | |||
| argumentDocs: | |||
| analyzer_name: '- (Required) Name of the Analyzer.' | |||
| arn: '- The Amazon Resource Name (ARN) of the Analyzer.' | |||
There was a problem hiding this comment.
(ARN) here shouldn't be removed. I think we should find & replace with the following criteria:
- Match
- (...)strings. - The match should be the 0th element of the string.
Description of your changes
Fixes https://github.com/upbound/official-providers/issues/297
With this PR, we add descriptions for fields and resources. These descriptions are taken from the provider metadata that is scraped from the TF registry by @ulucinar .
I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
make generateinprovider-awsValidated related descriptions for fields and crds.