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

Handle code blocks in import section #2382

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

andrzejressel
Copy link

Fix for pulumi/pulumi-docker#1201

I've tested it with docker provider and it seems to be working fine apart from this warning:

warning: unable to convert HCL example for Pulumi entity '#/resources/docker:index/serviceConfig:ServiceConfig'. The example will be dropped from any generated docs or SDKs: 1 error occurred:
        * [csharp, go, java, python, typescript, yaml] <nil>: pcl.BindProgram failed: example.pp:2,17-41: unknown package 'std'; unknown package 'std'; 

which removes example from ServiceConfig, but I think this it not related to this change.

@guineveresaenger
Copy link
Contributor

Hi @andrzejressel - thank you for your pull request. I'm trying to run tests for you but I'm seeing a setup issue on our end for the tests.
While we're waiting for that, I've run this change against one of our other providers (pulumi-aws), and I see an index out of bounds panic on line 1269. You may find the findCodeBlock function helpful here - also know that we're trying to move away from a line-by-line string-based parsing approach and use an AST-based Markdown parser instead, but haven't gotten to the import section yet.

Additionally, all Terraform code will need to be translated to Pulumi code. We do not, as a rule, display Terraform code in these docs.

This is an incredibly gnarly part of our codebase, so thank you so much for willing to get involved here! 💟 I'm happy to help and answer questions, you can also find me on the community Slack for Pulumi.

@guineveresaenger
Copy link
Contributor

I've rebased and we should be able to get some better test output here.

@andrzejressel
Copy link
Author

Should be fine now

@guineveresaenger
Copy link
Contributor

I will take a look next week - thank you for your patience!

Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

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

I've tried this against one of our other providers - pulumi-gcp, and I unfortunately still see a panic. There's something about the counter in the loop that's not quite right.

importString = fmt.Sprintf("%s %s", importString, p)
}
// Handle terraform blocks - pass them whole without changes
if trimmedSection == "```terraform" || trimmedSection == "```hcl" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use https://github.com/pulumi/pulumi-terraform-bridge/blob/master/pkg/tfgen/docs.go#L2353 for this check - it contains the current known detection patterns for TF code. 🙏

if strings.HasPrefix(trimmedSection, "```") {
initial := true
for {
section = subsection[i]
Copy link
Contributor

Choose a reason for hiding this comment

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

This line panics when running this code against pulumi-gcp.

@guineveresaenger
Copy link
Contributor

/run-acceptance-tests

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.

2 participants