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.

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

Removes logging #60

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions docs/developer_docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ use. If this is not given, Reporter will look for 'index\_pattern' to be set in
that is not configured, it will simply use gracc.osg.summary.
* vo (None): Virtual organization to run report on. Doesn't apply to most OSG reports
* template (None): HTML template file to use
* logfile (None): Logfile to use
* is_test (False): Test mode or not (If True, will send emails only to admins as set in config file
(test.emails and test.names)
* no_email (False): Don't send any emails at all. Just run the report
Expand Down Expand Up @@ -101,9 +100,6 @@ in the generation of the report.
try to use IndexPattern.indexpattern\_generate to create a more specific index pattern to optimize
query speed.
* check_no_email will look at the self.no_email flag, and if it's set, logs some info.
* get_logfile_path tries to set the logfile path to something that's valid for the user running the
report. It will try to set the logfile path to, respectively, the file given on the command line,
the path given in the configuration, the user's home directory, or the current working directory
* __establish_client is a hidden method, but I wanted to mention it because it is where the connection
to the GRACC host is established. It is not meant to be used in any reports.

Expand Down
2 changes: 1 addition & 1 deletion docs/example/SampleReport.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def main():
print "Yay, it worked!"
sys.exit(0)
except Exception as e:
ReportUtils.runerror(args.config, e, traceback.format_exc(), '/tmp/logfile.junk')
ReportUtils.runerror(args.config, e, traceback.format_exc())
sys.exit(1)

if __name__ == '__main__':
Expand Down
2 changes: 0 additions & 2 deletions docs/example/sample.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# General

default_logdir = '/var/log'

[elasticsearch]
hostname = 'https://gracc.opensciencegrid.org/q'
ok_statuses = ['green', 'yellow']
Expand Down
95 changes: 2 additions & 93 deletions gracc_reporting/ReportUtils.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class Reporter(object, metaclass=abc.ABCMeta):
:param str template: Filename of HTML template to inject report data into
:param bool is_test: Dry-run or real run
:param bool no_email: If true, don't send any emails
:param str logfile: Filename of log file for report
:param str althost_key: Alternate Elasticsearch Host key from config file.
Must be specified in [elasticsearch] section of
config file by name (e.g. my_es_cluster="https://hostname.me")
Expand All @@ -65,7 +64,6 @@ class Reporter(object, metaclass=abc.ABCMeta):
'index_key': 'index_pattern',
'vo': None,
'template': None,
'logfile': None,
'is_test': False,
'no_email': False,
'verbose': False
Expand Down Expand Up @@ -280,67 +278,6 @@ def check_no_email(self, emails):
else:
return False

def get_logfile_path(self, override_fn=None):
"""
Gets log file location. First tries user override, then tries config
file, then $HOME

:param str fn: Filename of logfile
:param bool override: Override this method by feeding in a logfile path
:return str: Path to logfile where we have permission to write
"""

if override_fn:
print("Writing log to {0}".format(override_fn))
return override_fn

try_locations = [os.path.expanduser('~')]

try:
logdir = self.config['default_logdir']
if logdir in try_locations:
try_locations.remove(logdir)
try_locations.insert(0, logdir)
except KeyError: # No entry in configfile
pass

dirname = 'gracc-reporting'
filename = '{0}.log'.format(self.report_type.lower())

for prefix in try_locations:
dirpath = os.path.join(prefix, dirname)
filepath = os.path.join(prefix, dirname, filename)

errmsg = "Couldn't write logfile to {0}. " \
"Moving to next path".format(filepath)

successmsg = "Writing log to {0}".format(filepath)

# Does the dir exist? If not, can we create it?
if not os.path.exists(dirpath):
# Try to make the logfile directory
try:
os.makedirs(dirpath)
except OSError as e: # Permission Denied or missing directory
print(e)
print(errmsg)
continue # Don't try to write somewhere we can't

# So dir exists. Can we write to the logfiles there?
try:
with open(filepath, 'a') as f:
f.write('')
except (IOError,
OSError) as e: # Permission Denied comes through as an IOError
print(e, '\n', errmsg)
else:
print(successmsg)
break
else:
# If none of the prefixes work for some reason, write to current working dir
filepath = os.path.join(os.getcwd(), filename)
return filepath

# Non-public methods

@staticmethod
Expand Down Expand Up @@ -523,24 +460,6 @@ def __setup_gen_logger(self):
else:
ch.setLevel(logging.WARNING)

if self.logfile is not None:
# FileHandler
fh = logging.FileHandler(self.logfile)
fh.setLevel(logging.DEBUG)

try:
f = ContextFilter(self.vo)
fh.addFilter(f)
logfileformat = logging.Formatter(
"%(asctime)s - %(name)s - %(levelname)s - %(vo)s - %(message)s")
except (NameError, AttributeError):
logfileformat = logging.Formatter(
"%(asctime)s - %(name)s - %(levelname)s - %(message)s")

fh.setFormatter(logfileformat)

logger.addHandler(fh)

# We only want one Stream Handler
for handler in logger.handlers:
if handler.__class__.__name__ == "StreamHandler":
Expand All @@ -554,23 +473,15 @@ def __setup_gen_logger(self):
return logger


def runerror(config, error, traceback, logfile):
def runerror(config, error, traceback):
"""
Global function to print, log, and email errors to admins

:param str config: Config filename
:param str error: Error raised
:param str traceback: Traceback from error
:param str logfile: Filename of logfile
:return None
"""
try:
with open(logfile, 'a') as f:
f.write(str(error))
except IOError: # Permission denied
reallogfile = os.path.join(os.path.expanduser('~'), logfile)
with open(reallogfile, 'a') as f:
f.write(str(error))
print(error, file=sys.stderr)

with open(config, 'r') as f:
Expand Down Expand Up @@ -649,9 +560,7 @@ def get_report_parser(no_time_options=False):
always_include.add_argument("-n", "--nomail", dest="no_email",
action="store_true", default=False,
help="Do not send email. ")
always_include.add_argument("-L", "--logfile", dest="logfile",
default=None, help="Specify non-standard location"
"for logfile")

if no_time_options:
return parser

Expand Down
3 changes: 2 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ opensearch-py
python-dateutil
toml
tabulate
pandas
pandas
setuptools
32 changes: 1 addition & 31 deletions tests/test_ReportUtils.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,35 +39,6 @@ def setUp(self):
self.r = FakeVOReport(vo='testVO')
self.r_copy = FakeVOReport(cfg_file=BAD_CONFIG_FILE)


class TestGetLogfilePath(TestReportUtilsBase):
"""Tests for ReportUtils.Reporter.get_logfile_path"""
def test_override(self):
"""Return override logfile if that's passed in"""
fn = "/tmp/override.log"
self.assertEqual(self.r.get_logfile_path(fn), fn)

def test_configfile(self):
"""Logfile should be set to configfile value"""
answer = os.path.join(self.r.config["default_logdir"],
'gracc-reporting', 'test.log')
self.assertEqual(self.r.get_logfile_path(), answer)

def test_fallback(self):
"""Set logdir to $HOME if no override and no configfile value"""
answer = os.path.join(os.path.expanduser('~'), 'gracc-reporting',
'test.log')
self.assertEqual(self.r_copy.get_logfile_path(), answer)

def test_bad_configval(self):
"""Set logdir to $HOME if configfile value is invalid"""
self.r_copy.config["default_logdir"] = '/'
answer = os.path.join(os.path.expanduser('~'), 'gracc-reporting',
'test.log')
self.assertEqual(self.r_copy.get_logfile_path(), answer)
del self.r_copy.config["default_logdir"]


class TestParseConfig(TestReportUtilsBase):
"""Tests for ReportUtils.Reporter._parse_config"""
def test_parse_config_control(self):
Expand Down Expand Up @@ -99,8 +70,7 @@ def test_parse_config_control(self):
'email': '[email protected]'
},
'smtphost': 'smtp.example.com'
},
'default_logdir': '/tmp/gracc-test'
}
}
self.assertDictEqual(self.r._parse_config(CONFIG_FILE), answer)

Expand Down
1 change: 0 additions & 1 deletion tests/test_config.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# General

default_logdir = '/tmp/gracc-test'
configured_vos = ['testvo']


Expand Down