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.

Sign up for GitHub

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

Jump to bottom

Use ~/.ccm/.dse.ini as dse credentials file if it exists by default. #426

Merged
merged 2 commits into from
Dec 17, 2015
Merged

Use ~/.ccm/.dse.ini as dse credentials file if it exists by default. #426

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5735094
Use ~/.ccm/.dse.ini as dse credentials file if it exists by default.
tolbertam Dec 16, 2015
5571363
Improve error validation for dse credentials.
tolbertam Dec 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Improve error validation for dse credentials.
  • Loading branch information
@tolbertam
tolbertam committed Dec 17, 2015
commit 5571363f0dd1c1f4b40db98e43f1f21ef646eccc
2 changes: 1 addition & 1 deletion ccmlib/cmds/cluster_cmds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def get_parser(self):
parser.add_option("--dse-password", type="string", dest="dse_password",
help="The password to use to download DSE with", default=None)
parser.add_option("--dse-credentials", type="string", dest="dse_credentials_file",
help="An ini-style config file containing the dse_username and dse_password under a dse_credentials section. [default to {0}/.dse.ini if it exists]".format(common.get_default_path_display_name()), default=None)
help="An ini-style config file containing the dse_username and dse_password under a dse_credentials section. [default to {}/.dse.ini if it exists]".format(common.get_default_path_display_name()), default=None)
parser.add_option("--install-dir", type="string", dest="install_dir",
help="Path to the cassandra or dse directory to use [default %default]", default="./")
parser.add_option('-n', '--nodes', type="string", dest="nodes",
Expand Down
24 changes: 18 additions & 6 deletions ccmlib/dse_cluster.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
import shutil
import signal
import subprocess
import sys
try:
import ConfigParser
except ImportError:
import configparser as ConfigParser

from six import iteritems
from six import iteritems, print_

from ccmlib import common, repository
from ccmlib.cluster import Cluster
Expand All @@ -25,6 +26,11 @@ def __init__(self, path, name, partitioner=None, install_dir=None, create_direct
self.dse_username = dse_username
if dse_password is not None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need to define self.dse_username and password as None? Couldn't this just be

self.load_credentials_from_file(dse_credentials_file)
if dse_username is not None:
    self.dse_username, self.dse_password = dse_username, dse_password

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There would be a possible issue if there is no credentials file and --dse-username and --dse-password were not passed in, here is the failure behavior with explicitly setting them to None beforehand:

ccmlib.common.ArgumentError: Invalid url http://downloads.datastax.com/enterprise/dse-4.8.2-bin.tar.gz (underlying error is: HTTP Error 401: Unauthorized)

And without:

AttributeError: 'DseCluster' object has no attribute 'dse_username'

I think the first scenario is more obvious, but maybe we should assert that there was a resolved username / password beforehand. I'll add some additional logic for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've improved this by logging warnings if either dse_username or password are not set:

Warning: No dse username detected, specify one using --dse-username or passing in a credentials file using --dse-credentials.
Warning: No dse password detected, specify one using --dse-password or passing in a credentials file using --dse-credentials.

The download will still be attempted (and thus the Invalid url error will appear), I figured i'd leave it as is in the event the download ever doesn't require authentication.

self.dse_password = dse_password

if self.dse_username is None:
print_("Warning: No dse username detected, specify one using --dse-username or passing in a credentials file using --dse-credentials.", file=sys.stderr)
if self.dse_password is None:
print_("Warning: No dse password detected, specify one using --dse-password or passing in a credentials file using --dse-credentials.", file=sys.stderr)
self.opscenter = opscenter
super(DseCluster, self).__init__(path, name, partitioner, install_dir, create_directory, version, verbose)

Expand All @@ -41,11 +47,17 @@ def load_credentials_from_file(self, dse_credentials_file):
creds_file = os.path.join(common.get_default_path(), '.dse.ini')
if os.path.isfile(creds_file):
dse_credentials_file = creds_file

parser = ConfigParser.ConfigParser()
parser.read(dse_credentials_file)
self.dse_username = parser.get('dse_credentials','dse_username')
self.dse_password = parser.get('dse_credentials','dse_password')

if dse_credentials_file is not None:
parser = ConfigParser.ConfigParser()
parser.read(dse_credentials_file)
if parser.has_section('dse_credentials'):
if parser.has_option('dse_credentials', 'dse_username'):
self.dse_username = parser.get('dse_credentials','dse_username')
if parser.has_option('dse_credentials', 'dse_password'):
self.dse_password = parser.get('dse_credentials','dse_password')
else:
print_("Warning: {} does not contain a 'dse_credentials' section.".format(dse_credentials_file), file=sys.stderr)

def hasOpscenter(self):
return os.path.exists(os.path.join(self.get_path(), 'opscenter'))
Expand Down