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

Skip reindexing for integration plugins index #92

Closed
wants to merge 1 commit into from
Closed

Skip reindexing for integration plugins index #92

wants to merge 1 commit into from

Conversation

jrodewig
Copy link

@jrodewig jrodewig commented Sep 14, 2023

Problem:
When adding a new integration plugin, the related plugin page in the Logstash ref links to a non-existent VPR docs landing page, causing broken links. Manually adding the landing page to the VPR docs doesn't currently work — it gets overwritten in the next VPR doc gen.

Example of this for an input plugin:

Solution:
Update the versioned_plugins.rb to avoid overwriting the integration plugin index in the VPR docs. This lets us manually add landing pages for integration plugins to the VPR docs. However, this means we'll need to manually maintain integrations-index.asciidoc going forward.

Testing:

  1. Check out the main branch locally.

  2. Run versiond_plugins.rb to generate the VPR docs:

    GITHUB_TOKEN=XXXXXXXXXXXXXX bundle exec ruby versioned_plugins.rb --output-path=<YOUR_PATH> --dry-run
  3. Include a landing page in the integrations-index.asciidoc file in the generated logstash-docs repo. The landing page doesn't need to exist. Example:

    ...
    include::integrations/rabbitmq-index.asciidoc[]
    
    include::integrations/just-another-index.asciidoc[]
  4. Check out this PR's branch locally.

  5. Rerun versiond_plugins.rb script to regenerate the VPR docs. Use the same output path.

    GITHUB_TOKEN=XXXXXXXXXXXXXX bundle exec ruby versioned_plugins.rb --output-path=<YOUR_PATH> --dry-run
  6. Verify that integrations-index.asciidoc file in the generated logstash-docs repo wasn't overwritten. It should still contain your include. Example:

    ...
    include::integrations/rabbitmq-index.asciidoc[]
    
    include::integrations/just-another-index.asciidoc[]

Related issue:

Closes #89

@jrodewig
Copy link
Author

jrodewig commented Sep 14, 2023

CC @karenzone @mashhurs I don't have permissions to add reviewers or labels. Can you take care of that for me (or grant me the permissions)?

Feel free to push commits to the PR or close it. Totally fine if we decide not to go with this approach. It was a good learning experience for me regardless. Thanks!

Copy link
Contributor

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

Maintaining integrations-index.asciidoc going forward isn't ideal. but wouldn't be a deal breaker because that would be a one-time thing.

We will need THESE integrations index pages updated programmatically: https://github.com/elastic/logstash-docs/blob/versioned_plugin_docs/docs/versioned-plugins/integrations/aws-index.asciidoc.

Our current workflow is a pain to set up, but after initial setup, it's maintenance-free.

@jrodewig
Copy link
Author

Maintaining integrations-index.asciidoc going forward isn't ideal. but wouldn't be a deal breaker because that would be a one-time thing.

Agree it's not ideal.

The only alternative I could think of would be an allowlist. That's harder to implement, and there would still be an additional maintenance step(s) to update that list for new integration plugins. This seems simpler. Open to other suggestions or ideas tho.

We will need THESE integrations index pages updated programmatically: https://github.com/elastic/logstash-docs/blob/versioned_plugin_docs/docs/versioned-plugins/integrations/aws-index.asciidoc.

Our current workflow is a pain to set up, but after initial setup, it's maintenance-free.

Yep. Those pages are left as is and will still auto-gen. This only impacts integrations-index.asciisoc.

@jrodewig jrodewig changed the title Skip reindexing for integration plugins Skip reindexing for integration plugins index Sep 15, 2023
@mashhurs
Copy link
Contributor

The proposed solution (skip integration type index skip) with this change helps to avoid overwrites unless we publish the plugin. If we publish the plugin type index page will be consistent.
@karenzone what if we add only plugin index (ex: docs/versioned-plugins/integrations/logstash-index.asciidoc) page and let type indexing happen after plugin publishing? As my understanding, it is because {type}s/{plugin_name}-index.asciidoc doesn't exist so we have broken links. If we create this placeholder, it seems to me problem is solved.

@jrodewig
Copy link
Author

Closing this PR. Feel free to re-open or reuse the code if it's helpful. Thanks!

@jrodewig jrodewig closed this Sep 21, 2023
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.

VPR: Revisit allowing a non-standard landing page (use case: new integrations)
3 participants