You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "nchammas (via GitHub)" <gi...@apache.org> on 2024/02/01 02:03:30 UTC

[PR] [SPARK-46935] Consolidate error documentation [spark]

nchammas opened a new pull request, #44971:
URL: https://github.com/apache/spark/pull/44971

   ### What changes were proposed in this pull request?
   
   Consolidate all error documentation into a single page of error states and conditions. Each condition and sub-condition will have a link anchor that allows for direct references.
   
   Here are some examples:
   
   ```
   sql-error-conditions.html#cannot-update-field
   sql-error-conditions.html#cannot-update-field-array-type
   sql-error-conditions.html#cannot-update-field-interval-type
   sql-error-conditions.html#cannot-update-field-map-type
   ```
   
   The table is styled to make it easier to read:
   - Sub-conditions are indented relative to their parent condition.
   - Long condition names like `UDTF_INVALID_ALIAS_IN_REQUESTED_ORDERING_STRING_FROM_ANALYZE_METHOD` wrap in a visually pleasing manner.
   
   The new documentation is generated dynamically via `docs/util/build-error-docs.py` as part of the documentation build. No generated files will be tracked in git.
   
   TODO:
   - [ ] Figure out what, if anything, we will do about the links we are breaking by deleting the old error pages.
   - [ ] Make sure we are using the correct terminology once SPARK-46810 is resolved.
   - [ ] Add opening prose for the main error conditions page.
   - [ ] Update the main Jekyll build to generate the new documentation on the fly.
   
   ### Why are the changes needed?
   
   The current error documentation has several problems that make it difficult to maintain and difficult to use:
   
   1. The current documentation consists of many Markdown files that are both programmatically generated and checked in to git. This combination leads to problems like the checked in files getting out of sync with the source they are generated from, and leads to unnecessary code churn and problems like the one that precipitated #44847.
   2. The individual pages like `docs/sql-error-conditions-cannot-load-state-store-error-class.md` are sparse and don't add much value as standalone pages. They also clutter the top-level namespace. There are around 60 of these individual pages, one for each error condition that has sub-conditions.
   
       The number of pages to manage leads to other, more subtle problems as well. Though there are 60 individual pages, only 22 of them are captured in `docs/_data/menu-sql.yaml`.
   3. The current process to generate this documentation is awkward. We are using a) a Spark Scala test that requires b) a special environment variable to be set so that c) we read some JSON, in order to d) generate Markdown files, which in turn e) get compiled into HTML.
   
       Fundamentally, we are just generating HTML from JSON. It seems inappropriate to couple this process to the Spark build.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, it overhauls the error documentation. Unless we add redirects, several links that work against the Spark 3.5 documentation will no longer work against the Spark 4.0 documentation.
   
   ### How was this patch tested?
   
   I built the docs and reviewed the results in my browser.
   
   <img width="400" src="https://github.com/apache/spark/assets/1039369/effdeea9-a5f8-4471-873f-9746480e1b97">
   <br>
   <img width="400" src="https://github.com/apache/spark/assets/1039369/278af3de-a6a3-4872-84ee-97371f92232e">
   <br>
   <img width="400" src="https://github.com/apache/spark/assets/1039369/e11dc3bf-bfda-4d70-bd3d-605d48d06d26">
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1920365869

   > @MaxGekk @itholic @srielau - I consider this PR blocked by [SPARK-46810](https://issues.apache.org/jira/browse/SPARK-46810), but I want to share it with you nonetheless to get your feedback.
   The problem with a table is that both the error conditions and the descriptions can be lengthy.
   I do think we want to keep the option of "custom crafted" detail messages (although that does not conflict with what you propose).
   Either way I agree that anything that makes it more automateable to maintain the  page is a good thing.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1930079352

   > I'd like to keep the option open for hand crafted long-form error docs. Any thoughts?
   
   I'm fine with that. If we find in the future that some error condition merits a dedicated page, then what I suggest we do is the following:
   - Create a hand-crafted error page with the extended prose we want.
   - Add a link from the error condition's message in the JSON document to that standalone page.
   
       We should express that link using Markdown. `build-error-docs.py` [compiles any Markdown in the message][1] into clean HTML.
   
   [1]: https://github.com/apache/spark/pull/44971/files#diff-b957f1b0f8f56d693a8b175c6068e361b3f3e4a5ba5b971604702f9adfe5df85R24
   
   The result will be a nice HTML link from the description of the error condition in the master table to whatever error page we need.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on code in PR #44971:
URL: https://github.com/apache/spark/pull/44971#discussion_r1568942970


##########
docs/util/build-error-docs.py:
##########
@@ -0,0 +1,151 @@
+"""
+Generate a unified page of documentation for all error conditions.
+"""
+import json
+import os
+import re
+from itertools import chain
+from pathlib import Path
+from textwrap import dedent
+
+# To avoid adding new direct dependencies, we import from within mkdocs.
+# This is not ideal as unrelated updates to mkdocs may break this script.
+from mkdocs.structure.pages import markdown
+
+THIS_DIR = Path(__file__).parent
+SPARK_PROJECT_ROOT = THIS_DIR.parents[1]
+DOCS_ROOT = SPARK_PROJECT_ROOT / "docs"
+ERROR_CLASSES_PATH = SPARK_PROJECT_ROOT / "common/utils/src/main/resources/error/error-classes.json"
+
+
+def assemble_message(message_parts):
+    message = " ".join(message_parts)
+    cleaned_message = re.sub(r"(<.*?>)", lambda x: f"`{x.group(1)}`", message)
+    return markdown.markdown(cleaned_message)
+
+
+def load_error_classes(path):
+    with open(path) as f:
+        raw_error_classes = json.load(f)
+    error_classes = dict()
+    for name, details in raw_error_classes.items():
+        if name.startswith("_LEGACY_ERROR") or name.startswith("INTERNAL_ERROR"):
+            continue
+        if "subClass" in details:
+            for sub_name in details["subClass"]:
+                details["subClass"][sub_name]["message"] = (
+                    assemble_message(details["subClass"][sub_name]["message"])
+                )
+        details["message"] = assemble_message(details["message"])
+        error_classes[name] = details
+    return error_classes
+
+
+def anchor_name(class_name: str, sub_class_name: str = None):
+    """
+    URLs can, in practice, be up to 2,000 characters long without causing any issues. So
+    we preserve the class name most as-is for use in the anchor, even when that name is very
+    long.
+    See: https://stackoverflow.com/a/417184
+    """
+    parts = [
+        part for part in (class_name, sub_class_name)
+        if part
+    ]
+    anchor = "-".join(parts).lower().replace("_", "-")
+    return anchor
+
+
+def generate_doc_rows(class_name, class_details):
+    class_row = [
+        """
+        <tr id="{anchor}">
+            <td>{sql_state}</td>
+            <td>
+                <span class="error-name">
+                    <code>
+                    <a href="#{anchor}">#</a>
+                    </code>
+                    {class_name}
+                </span>
+            </td>
+            <td>{message}</td>
+        </tr>
+        """
+        .format(
+            anchor=anchor_name(class_name),
+            sql_state=class_details["sqlState"],
+            # This inserts soft break opportunities so that if a long name needs to be wrapped
+            # it will wrap in a visually pleasing manner.
+            # See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/wbr
+            class_name=class_name.replace("_", "<wbr />_"),
+            message=class_details["message"],
+        )
+    ]
+    sub_class_rows = []
+    if "subClass" in class_details:
+        for sub_class_name in sorted(class_details["subClass"]):
+            sub_class_rows.append(
+                """
+                <tr id="{anchor}">
+                    <td></td>
+                    <td class="error-sub-class">
+                        <span class="error-name">
+                            <code>
+                            <a href="#{anchor}">#</a>
+                            </code>
+                            {sub_class_name}
+                        </span>
+                    </td>
+                    <td class="error-sub-class">{message}</td>
+                </tr>
+                """
+                .format(
+                    anchor=anchor_name(class_name, sub_class_name),
+                    # See comment above for explanation of `<wbr />`.
+                    sub_class_name=sub_class_name.replace("_", "<wbr />_"),
+                    message=class_details["subClass"][sub_class_name]["message"],
+                )
+            )
+    doc_rows = class_row + sub_class_rows
+    return [
+        dedent(row).strip()
+        for row in doc_rows
+    ]
+
+
+def generate_doc_table(error_classes):
+    doc_rows = chain.from_iterable([
+        generate_doc_rows(class_name, class_details)
+        for class_name, class_details
+        in sorted(
+            error_classes.items(),
+            key=lambda x: (x[1]["sqlState"], x[0]),
+        )
+    ])
+    table_html = (
+        """
+        <table id="error-conditions">
+        <tr>
+            <th>Error State / SQLSTATE</th>
+            <th>Error Condition & Sub-Condition</th>
+            <th>Message</th>
+        </tr>
+        {rows}
+        </table>
+        """
+    )
+    # We dedent here rather than above so that the interpolated rows (which are not
+    # indented) don't prevent the dedent from working.
+    table_html = dedent(table_html).strip().format(rows="\n".join(list(doc_rows)))
+    return table_html
+
+
+if __name__ == "__main__":

Review Comment:
   Correct, this is the main goal of this PR: to remove auto-generated content from git, plus all the associated friction it causes.
   
   One of my [TODOs for this PR][1] is to invoke this script from the Jekyll build. In other words, no-one will need to remember to run anything separately. They just run `bundle exec jekyll build` as they normally do.
   
   [1]: https://github.com/apache/spark/pull/44971#issue-2111332887



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1927895114

   > @MaxGekk @itholic - What do you think of consolidating the error documentation as demonstrated here?
   > 
   > @srielau - Do you still think we need standalone pages for each error condition? Curious to know if the screenshots I posted changed your mind. If you're still concerned about lengthy error names or messages, please share some examples so I can check how they are rendered.
   
   I'm fien with that proposal. I'd like to keep the option open for hand crafted long-form error docs. Any thoughts?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-2061538407

   This is ready for review. The only two things I'll call out specifically for additional attention are this open TODO:
   
   >  Figure out what, if anything, we will do about the links we are breaking by deleting the old error pages.
   
   And this minor change to our workflow for building docs:
   
   > One developer-facing change included in this PR is that the main documentation build with Jekyll now depends on our Python development environment. This is in order to generate the error documentation, since that depends on our ability to render Markdown. Previously, only the SQL and Python API docs depended on the Python environment.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-2062874072

   > And this minor change to our workflow for building docs:
   
   I think this is fine. During release process, we need to generate all docs so Python development environment is already needed.
   
   cc @HyukjinKwon as well.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-2065652709

   thanks, merging to master!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1920356171

   @MaxGekk @itholic @srielau - I consider this PR blocked by SPARK-46810, but I want to share it with you nonetheless to get your feedback.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #44971:
URL: https://github.com/apache/spark/pull/44971#discussion_r1568388171


##########
docs/util/build-error-docs.py:
##########
@@ -0,0 +1,151 @@
+"""
+Generate a unified page of documentation for all error conditions.
+"""
+import json
+import os
+import re
+from itertools import chain
+from pathlib import Path
+from textwrap import dedent
+
+# To avoid adding new direct dependencies, we import from within mkdocs.
+# This is not ideal as unrelated updates to mkdocs may break this script.
+from mkdocs.structure.pages import markdown
+
+THIS_DIR = Path(__file__).parent
+SPARK_PROJECT_ROOT = THIS_DIR.parents[1]
+DOCS_ROOT = SPARK_PROJECT_ROOT / "docs"
+ERROR_CLASSES_PATH = SPARK_PROJECT_ROOT / "common/utils/src/main/resources/error/error-classes.json"
+
+
+def assemble_message(message_parts):
+    message = " ".join(message_parts)
+    cleaned_message = re.sub(r"(<.*?>)", lambda x: f"`{x.group(1)}`", message)
+    return markdown.markdown(cleaned_message)
+
+
+def load_error_classes(path):
+    with open(path) as f:
+        raw_error_classes = json.load(f)
+    error_classes = dict()
+    for name, details in raw_error_classes.items():
+        if name.startswith("_LEGACY_ERROR") or name.startswith("INTERNAL_ERROR"):
+            continue
+        if "subClass" in details:
+            for sub_name in details["subClass"]:
+                details["subClass"][sub_name]["message"] = (
+                    assemble_message(details["subClass"][sub_name]["message"])
+                )
+        details["message"] = assemble_message(details["message"])
+        error_classes[name] = details
+    return error_classes
+
+
+def anchor_name(class_name: str, sub_class_name: str = None):
+    """
+    URLs can, in practice, be up to 2,000 characters long without causing any issues. So
+    we preserve the class name most as-is for use in the anchor, even when that name is very
+    long.
+    See: https://stackoverflow.com/a/417184
+    """
+    parts = [
+        part for part in (class_name, sub_class_name)
+        if part
+    ]
+    anchor = "-".join(parts).lower().replace("_", "-")
+    return anchor
+
+
+def generate_doc_rows(class_name, class_details):
+    class_row = [
+        """
+        <tr id="{anchor}">
+            <td>{sql_state}</td>
+            <td>
+                <span class="error-name">
+                    <code>
+                    <a href="#{anchor}">#</a>
+                    </code>
+                    {class_name}
+                </span>
+            </td>
+            <td>{message}</td>
+        </tr>
+        """
+        .format(
+            anchor=anchor_name(class_name),
+            sql_state=class_details["sqlState"],
+            # This inserts soft break opportunities so that if a long name needs to be wrapped
+            # it will wrap in a visually pleasing manner.
+            # See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/wbr
+            class_name=class_name.replace("_", "<wbr />_"),
+            message=class_details["message"],
+        )
+    ]
+    sub_class_rows = []
+    if "subClass" in class_details:
+        for sub_class_name in sorted(class_details["subClass"]):
+            sub_class_rows.append(
+                """
+                <tr id="{anchor}">
+                    <td></td>
+                    <td class="error-sub-class">
+                        <span class="error-name">
+                            <code>
+                            <a href="#{anchor}">#</a>
+                            </code>
+                            {sub_class_name}
+                        </span>
+                    </td>
+                    <td class="error-sub-class">{message}</td>
+                </tr>
+                """
+                .format(
+                    anchor=anchor_name(class_name, sub_class_name),
+                    # See comment above for explanation of `<wbr />`.
+                    sub_class_name=sub_class_name.replace("_", "<wbr />_"),
+                    message=class_details["subClass"][sub_class_name]["message"],
+                )
+            )
+    doc_rows = class_row + sub_class_rows
+    return [
+        dedent(row).strip()
+        for row in doc_rows
+    ]
+
+
+def generate_doc_table(error_classes):
+    doc_rows = chain.from_iterable([
+        generate_doc_rows(class_name, class_details)
+        for class_name, class_details
+        in sorted(
+            error_classes.items(),
+            key=lambda x: (x[1]["sqlState"], x[0]),
+        )
+    ])
+    table_html = (
+        """
+        <table id="error-conditions">
+        <tr>
+            <th>Error State / SQLSTATE</th>
+            <th>Error Condition & Sub-Condition</th>
+            <th>Message</th>
+        </tr>
+        {rows}
+        </table>
+        """
+    )
+    # We dedent here rather than above so that the interpolated rows (which are not
+    # indented) don't prevent the dedent from working.
+    table_html = dedent(table_html).strip().format(rows="\n".join(list(doc_rows)))
+    return table_html
+
+
+if __name__ == "__main__":

Review Comment:
   when do we run this script? Now the generated doc pages are not committed in the codebase.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "mihailom-db (via GitHub)" <gi...@apache.org>.
mihailom-db commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-2069173302

   This PR likely broke master and Linter CI.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #44971: [SPARK-46935][DOCS] Consolidate error documentation
URL: https://github.com/apache/spark/pull/44971


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1927283362

   @MaxGekk @itholic - What do you think of consolidating the error documentation as demonstrated here?
   
   @srielau - Do you still think we need standalone pages for each error condition? Curious to know if the screenshots I posted changed your mind.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1920376018

   > The problem with a table is that both the error conditions and the descriptions can be lengthy.
   
   The generated table renders very nicely for me. I spent some time on styling the table appropriately to solve this problem.
   
   Take a look at the screenshots in the PR description. One of them shows how the longest error condition name we have soft wraps nicely.
   
   What are some examples of error conditions you think won't display well in the proposed documentation? I am happy to share additional screenshots to show you how they render.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-2058128551

   let's fix conflicts and move forward, thanks for the work!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "srielau (via GitHub)" <gi...@apache.org>.
srielau commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1992549808

   > The discussion on [SPARK-46810](https://issues.apache.org/jira/browse/SPARK-46810) is not moving forward, unfortunately.
   > 
   > I'd like to get this in as it is a non-trivial maintenance win. If [SPARK-46810](https://issues.apache.org/jira/browse/SPARK-46810) gets going again, we can always rename things on the consolidated error page as necessary, and it will be much easier to do so.
   > 
   > @srielau @MaxGekk - If I resolve the merge conflicts, can we move this PR forward?
   
   +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-1992117020

   The discussion on SPARK-46810 is not moving forward, unfortunately.
   
   I'd like to get this in as it is a non-trivial maintenance win. If SPARK-46810 gets going again, we can always rename things on the consolidated error page as necessary, and it will be much easier to do so.
   
   @srielau @MaxGekk - If I resolve the merge conflicts, can we move this PR forward?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46935][DOCS] Consolidate error documentation [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44971:
URL: https://github.com/apache/spark/pull/44971#issuecomment-2052098166

   > The discussion on [SPARK-46810](https://issues.apache.org/jira/browse/SPARK-46810) is not moving forward, unfortunately.
   
   This discussion has since been resolved in favor of Option 1, described on the ticket.
   
   I will update this PR to resolve conflicts. I still need, however, a committer to give the go-ahead for this restructuring of our error documentation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org