-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: readme things that didnt work for me #129
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe README for Gmail API integration with MindsDB was enhanced with additional guidance on OAuth client ID creation, including an alternative application type option. It added detailed instructions for placing the Gmail API credentials JSON inside the MindsDB Docker container, with an example Docker run command. The example SQL command was updated to reference the correct credentials path inside the container. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
mindsdb-mcp/README.md (1)
212-212
: Improve clarity and grammar in the OAuth client instructions.The sentence is missing a comma before “and,” and “setup” (noun) should be “set up” (verb). Consider rewriting for readability:
- - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes it might not work straightforward and you need to setup a `Desktop application` instead. + - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes it might not work straightforwardly, and you need to set up a `Desktop application` instead.🧰 Tools
🪛 LanguageTool
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup aDesktop application
instead. -...(NOUN_VERB_CONFUSION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mindsdb-mcp/README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
mindsdb-mcp/README.md
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...
(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup a Desktop application
instead. -...
(NOUN_VERB_CONFUSION)
🔇 Additional comments (1)
mindsdb-mcp/README.md (1)
224-230
: Approve the Docker mount example for credentials.The
-v
flag example clearly demonstrates how to mount the downloaded JSON into the container, which will help users avoid manual copy-paste pitfalls when placing credentials inside MindsDB.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
mindsdb-mcp/README.md (1)
212-212
: Improve grammar and clarity in OAuth client ID note.The sentence is missing a comma before “and” and uses “setup” incorrectly as a verb. Consider rephrasing for readability.
- - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes it might not work straightforward and you need to setup a `Desktop application` instead. + - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes this may not work, and you need to set up a `Desktop application` instead.🧰 Tools
🪛 LanguageTool
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup aDesktop application
instead. -...(NOUN_VERB_CONFUSION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mindsdb-mcp/README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
mindsdb-mcp/README.md
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...
(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup a Desktop application
instead. -...
(NOUN_VERB_CONFUSION)
mindsdb-mcp/README.md
Outdated
PARAMETERS = { | ||
"credentials_file": "/mindsdb/mindsdb/integrations/handlers/gmail_handler/client_secret.json", | ||
"scopes": ['https://.../gmail.compose', 'https://.../gmail.readonly'] | ||
}; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate PARAMETERS
block in SQL snippet.
The current snippet has two PARAMETERS = {
lines and an extra closing brace, which is invalid SQL/JSON syntax. Consolidate into a single block:
-```SQL
-CREATE DATABASE mindsdb_gmail
-WITH ENGINE = 'gmail',
-PARAMETERS = {
-PARAMETERS = {
- "credentials_file": "/mindsdb/mindsdb/integrations/handlers/gmail_handler/client_secret.json",
- "scopes": ['https://.../gmail.compose', 'https://.../gmail.readonly']
-};
-};
-```
+```sql
+CREATE DATABASE mindsdb_gmail
+WITH ENGINE = 'gmail',
+PARAMETERS = {
+ "credentials_file": "/mindsdb/mindsdb/integrations/handlers/gmail_handler/client_secret.json",
+ "scopes": ['https://.../gmail.compose', 'https://.../gmail.readonly']
+};
+```
🤖 Prompt for AI Agents
In mindsdb-mcp/README.md around lines 245 to 249, the SQL snippet incorrectly
contains a duplicate PARAMETERS block and an extra closing brace, causing
invalid syntax. Remove the duplicate PARAMETERS = { line and the extra closing
brace so that there is only one PARAMETERS block enclosing the credentials_file
and scopes entries. Ensure the snippet starts with PARAMETERS = { and ends with
a single closing brace followed by a semicolon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
mindsdb-mcp/README.md (2)
224-230
: Fix port mapping mismatch in Docker run exampleThe example still maps the MCP API to port
47335
, but earlier instructions and the MCP server use47337
. Update to ensure the container’s port matches the rest of the guide.- -p 47334:47334 -p 47335:47335 \ + -p 47334:47334 -p 47337:47337 \
245-249
: Remove duplicatePARAMETERS
block and extra closing brace in SQL snippetThere are two
PARAMETERS = {
lines and an extra};
which make the SQL invalid. Consolidate into a single block.-PARAMETERS = { -PARAMETERS = { +PARAMETERS = { "credentials_file": "/mindsdb/mindsdb/integrations/handlers/gmail_handler/client_secret.json", "scopes": ['https://.../gmail.compose', 'https://.../gmail.readonly'] -}; -}; +};
🧹 Nitpick comments (1)
mindsdb-mcp/README.md (1)
212-212
: Refine wording and correct verb formThe line uses “setup” (noun) instead of “set up” (verb) and could benefit from clearer adverbial phrasing and a comma before “and.”
- - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes it might not work straightforward and you need to setup a `Desktop application` instead. + - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes it might not work straightforwardly, and you need to set up a `Desktop application` instead.🧰 Tools
🪛 LanguageTool
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup aDesktop application
instead. -...(NOUN_VERB_CONFUSION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mindsdb-mcp/README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
mindsdb-mcp/README.md
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...
(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup a Desktop application
instead. -...
(NOUN_VERB_CONFUSION)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
mindsdb-mcp/README.md (1)
224-230
: Address port mapping mismatch in Docker run example.The snippet still maps port
47335
instead of47337
, which will prevent the MCP server on47337
from being reachable (see line 66). This issue was noted in a previous review. Apply this diff:- -p 47334:47334 -p 47335:47335 \ + -p 47334:47334 -p 47337:47337 \
🧹 Nitpick comments (2)
mindsdb-mcp/README.md (2)
212-212
: Correct grammar and verb usage in OAuth instructions.The sentence is missing a comma after “straightforward” and uses “setup” (noun) instead of the verb “set up.” Consider rephrasing:
- - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes it might not work straightforward and you need to setup a `Desktop application` instead. + - Now, create the OAuth client ID. Choose `Web application` for the `Application Type` and give it a name. Sometimes it might not work straightforwardly, and you need to set up a `Desktop application` instead.🧰 Tools
🪛 LanguageTool
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup aDesktop application
instead. -...(NOUN_VERB_CONFUSION)
251-252
: Fix parameter naming and spelling in scopes note.The term “scoped” is unclear, and “ommitted” is misspelled. Consider:
-Note that in some cases, scoped will need to be set to `"scopes": ['https://www.googleapis.com/auth/gmail.readonly', -'https://www.googleapis.com/auth/gmail.compose']` or can be entirely ommitted. +Note that in some cases, the scopes parameter may need to be set to: +```json + "scopes": [ + "https://www.googleapis.com/auth/gmail.readonly", + "https://www.googleapis.com/auth/gmail.compose" + ] +``` +or can be entirely omitted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mindsdb-mcp/README.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
mindsdb-mcp/README.md
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etimes it might not work straightforward and you need to setup a `Desktop applicatio...
(COMMA_COMPOUND_SENTENCE)
[grammar] ~212-~212: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ot work straightforward and you need to setup a Desktop application
instead. -...
(NOUN_VERB_CONFUSION)
[uncategorized] ~250-~250: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ou added to the Test users
in your app and you are good to go. Note that in some c...
(COMMA_COMPOUND_SENTENCE)
🔇 Additional comments (1)
mindsdb-mcp/README.md (1)
245-245
: Correct JSON syntax in SQLPARAMETERS
block.The comma after the
credentials_file
entry is present, ensuring valid JSON. This aligns with the earlier recommendation to fix the missing comma.
loved your tutorial!
added some things is stumbled upon and might help readers
Summary by CodeRabbit