Skip to content
This repository has been archived by the owner on Aug 16, 2022. It is now read-only.

SQL plugin docs for basic usage #146

Merged
merged 9 commits into from
Dec 5, 2019
Merged

SQL plugin docs for basic usage #146

merged 9 commits into from
Dec 5, 2019

Conversation

dai-chen
Copy link
Contributor

@dai-chen dai-chen commented Dec 3, 2019

Issue #, if available:

Description of changes: Adding 4 new documents for basic usage of SQL plugin which are generated from template and test code in issue opendistro-for-elasticsearch/sql#293.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@aetter
Copy link
Contributor

aetter commented Dec 5, 2019

Hey @dai-chen, this looks really good. Any chance you can use fenced code blocks with highlighting rather than the indented version, or is that a high LOE given the tools you're using?

```json
{
  "error": {
    "reason": "Invalid SQL query",
    "details": "Field [unknown] cannot be found or used here.",
    "type": "SemanticAnalysisException"
  },
  "status": 400
}
```

@dai-chen
Copy link
Contributor Author

dai-chen commented Dec 5, 2019

Hey @dai-chen, this looks really good. Any chance you can use fenced code blocks with highlighting rather than the indented version, or is that a high LOE given the tools you're using?

```json
{
  "error": {
    "reason": "Invalid SQL query",
    "details": "Field [unknown] cannot be found or used here.",
    "type": "SemanticAnalysisException"
  },
  "status": 400
}

Hi @aetter, thanks for the review! Sure, will change it. I'm still new to Pandoc. Do you happen to know if this is supported by Pandoc? The docs in this PR were just converted from ReST docs in my opendistro-for-elasticsearch/sql#302. If no luck, I can do it manually. Thanks!

@aetter
Copy link
Contributor

aetter commented Dec 5, 2019

@dai-chen, so it looks to me like Pandoc prefers indented code blocks for whatever reason, but if you specify a language, it uses fenced blocks. So rather than:

SQL query::

   >> curl -H 'Content-Type: application/json' -X POST localhost:9200/_opendistro/_sql -d '{
     "query" : "SELECT * FROM accounts"
   }'

Try:

SQL query:

.. code:: sql (or bash or json)

   >> curl -H 'Content-Type: application/json' -X POST localhost:9200/_opendistro/_sql -d '{
     "query" : "SELECT * FROM accounts"
   }'

Unfortunately, I don't see any way to instruct Pandoc to just prefer fenced blocks, so I think adding a language and making the RST a little more complicated might be the easiest path forward.

@dai-chen
Copy link
Contributor Author

dai-chen commented Dec 5, 2019

@aetter Thanks! Good to know this. Will do it manually this time and change my document generator for future use.

@dai-chen
Copy link
Contributor Author

dai-chen commented Dec 5, 2019

@aetter I've finished the changes. Because I don't know how it looks on GitHub Pages, I just check the how it's rendered on GitHub. If possible, please help double check if any more changes needed. Thanks!

@aetter
Copy link
Contributor

aetter commented Dec 5, 2019

Awesome, thanks a bunch @dai-chen. The only issue I'm seeing now is that the code is unnecessarily indented and harder to copy and paste, but I'll fix it manually post-merge and trust that you'll handle it on your side for the next content drop. 👍

@aetter aetter merged commit 510d596 into opendistro:master Dec 5, 2019
@dai-chen
Copy link
Contributor Author

dai-chen commented Dec 5, 2019

Awesome, thanks a bunch @dai-chen. The only issue I'm seeing now is that the code is unnecessarily indented and harder to copy and paste, but I'll fix it manually post-merge and trust that you'll handle it on your side for the next content drop. 👍

Thanks a lot! Will change my doc generator accordingly. :)

@aetter
Copy link
Contributor

aetter commented Dec 5, 2019

No worries! Cleanup should be live now: 21c742d

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants