-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
feature: add couchbase query runner #3658
feature: add couchbase query runner #3658
Conversation
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.
Thanks, @AntonZarutsky ! Please see comments.
redash/query_runner/couchbase.py
Outdated
def test_connection(self): | ||
result = self.call_service(self.noop_query, '') | ||
|
||
def get_buckets(self, query, nameParam): |
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.
nameParam
should be name_param
to be consistent with PEP-8/our naming convention.
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.
@arikfr. thx for review. Fixed nameParams.
redash/query_runner/couchbase.py
Outdated
port = self.configuration.get('port', 8095) | ||
params = {'statement': query} | ||
|
||
url = "http://%s:%s/query/service" % (host, port) |
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.
Does Couchbase support HTTPS? In such case, we should probably make the protocol (http) configurable.
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.
extracted as parameter
Hi @arikfr. Updated PR. Can you check if we can proceed further? |
Merged. Thanks! |
* feature: add couchbase query runner * fix style * fix style * fix style * fix naming due to convention * extracting protocol as parameter
What type of PR is this? (check all applicable)
Description
Connector for Couchbase 5x+
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)