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

[SPARK-46437][DOCS] Add custom tags for conditional Jekyll includes #44630

Closed

Conversation

nchammas
Copy link
Contributor

@nchammas nchammas commented Jan 8, 2024

What changes were proposed in this pull request?

Add custom Jekyll tags to enable us to conditionally include files in our documentation build in a more user-friendly manner. This example demonstrates how a custom tag can build on one of Jekyll's built-in tags.

Without this change, files have to be included as follows:

{% for static_file in site.static_files %}
    {% if static_file.name == 'generated-agg-funcs-table.html' %}
        {% include_relative generated-agg-funcs-table.html %}
        {% break %}
    {% endif %}
{% endfor %}

With this change, they can be included more intuitively in one of two ways:

{% include_relative_if_exists generated-agg-funcs-table.html %}
{% include_api_gen generated-agg-funcs-table.html %}

include_relative_if_exists includes a file if it exists and substitutes an HTML comment if not. Use this tag when it's always OK for an include not to exist.

include_api_gen includes a file if it exists. If it doesn't, it tolerates the missing file only if one of the SKIP_ flags is set. Otherwise it raises an error. Use this tag for includes that are generated for the language APIs. These files are required to generate complete documentation, but we tolerate their absence during development---i.e. when a skip flag is set.

include_api_gen will place a visible text placeholder in the document and post a warning to the console to indicate that missing API files are being tolerated.

$ SKIP_API=1 bundle exec jekyll build
Configuration file: /Users/nchammas/dev/nchammas/spark/docs/_config.yml
            Source: /Users/nchammas/dev/nchammas/spark/docs
       Destination: /Users/nchammas/dev/nchammas/spark/docs/_site
 Incremental build: disabled. Enable with --incremental
      Generating... 
Warning: Tolerating missing API files because the following skip flags are set: SKIP_API
                    done in 1.703 seconds.
 Auto-regeneration: disabled. Use --watch to enable.

This PR supersedes #44393.

Why are the changes needed?

Jekyll does not have a succinct way to check if a file exists, so the required directives to implement such functionality are very cumbersome.

We need the ability to do this so that we can build the docs successfully with SKIP_API=1, since many includes reference files that are only generated when SKIP_API is not set.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manually building and reviewing the docs, both with and without SKIP_API=1.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the DOCS label Jan 8, 2024
@nchammas nchammas changed the title [SPARK-46437][DOCS] Add custom tag for conditional Jekyll includes [SPARK-46437][DOCS] Add custom tags for conditional Jekyll includes Jan 9, 2024
@nchammas nchammas marked this pull request as ready for review January 9, 2024 19:33
@nchammas
Copy link
Contributor Author

nchammas commented Jan 9, 2024

Documentation build passing here.

cc @HyukjinKwon - These new Jekyll tags properly solve the problem described here.

I will also be using them extensively if/when I get committer buy-in on #44300.

@HyukjinKwon
Copy link
Member

Merged to master.

@nchammas nchammas deleted the SPARK-46437-conditional-jekyll-include branch January 10, 2024 05:06
HyukjinKwon pushed a commit that referenced this pull request Jan 10, 2024
### What changes were proposed in this pull request?

As part of #44630 I neglected to update some places that still use the following Liquid directive pattern:

```liquid
{% for static_file in site.static_files %}
    {% if static_file.name == 'generated-agg-funcs-table.html' %}
        {% include_relative generated-agg-funcs-table.html %}
        {% break %}
    {% endif %}
{% endfor %}
```

This PR replaces all remaining instances of this pattern with the new `include_api_gen` Jekyll tag.

### Why are the changes needed?

For consistency in how we build our docs.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Manually building and reviewing the configuration docs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #44663 from nchammas/configuration-include-api-gen.

Authored-by: Nicholas Chammas <nicholas.chammas@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants