work #25
work #25
Conversation
ghost
commented
Sep 21, 2020
•
ghost
commented
- Work out number of errors in Scrapy log file
- Consider number of errors in decision to backup or not
- Don't backup subsets (sample, date filters)
- Don't backup if data files missing
- Add Sentry
- Add test run option
- Delete files we make and log and data files
|
Making draft while adding more things; will take out of draft when ready for review |
ghost
marked this pull request as draft
ghost
force-pushed
the
james-work-in-progress-4
branch
from
39fb570 to
1c72b6d
Compare
Work out number of errors in Scrapy log file Consider number of errors in decision to backup or not Don't backup subsets (sample, date filters) Don't backup if data files missing Add Sentry Don't check deleted collections
ghost
force-pushed
the
james-work-in-progress-4
branch
from
1c72b6d to
a319064
Compare
ghost
force-pushed
the
james-work-in-progress-4
branch
4 times, most recently
from
013366a to
b031cdf
Compare
ghost
force-pushed
the
james-work-in-progress-4
branch
from
b031cdf to
b782b5c
Compare
ghost
marked this pull request as ready for review
|
@jpmckinney Now ready for review - thanks |
|
@jpmckinney Can you look at this? Thanks |
ocdskingfisherarchive/archive.py
Outdated
| print( | ||
| "Collection " + str(collection.database_id) + " result: " + ("Archive" if should_archive else "Leave") | ||
| ) |
There was a problem hiding this comment.
Instead of both logging and printing, you should just add appropriate handlers.
There was a problem hiding this comment.
If we are sure we want logging messages to go to console, then yes. However I remembered some chat about minimising noisy cron's, which would mean we don't. In that case having a print is good, so the operator can see results straight away without having to then go and look in the log files. That was my thinking behind putting the print in.
ocdskingfisherarchive/collection.py
Outdated
| data_dir = self.config.directory_data + '/' + self.source_id + '/' + \ | ||
| self.data_version.strftime("%Y%m%d_%H%M%S") | ||
| # We use os.system here so we know the exact command so we can set up sudo correctly | ||
| return1 = os.system('sudo -u ocdskfs /bin/rm -rf '+data_dir) |
There was a problem hiding this comment.
Why not just set things up so that the ocdskfs user runs the archive command? Seems like a lot of trouble otherwise (changes to the sudo command require changes to deploy...).
There was a problem hiding this comment.
So this is actually matching the current setup as closely as possible, to minimise changes on server. We already have sudo set up: https://github.com/open-contracting/deploy/blob/master/salt/ocdskingfisherarchive/archive.sudoers
And also noting that we have to have some salt changes when deploying this (config file, logging config file, new pillar variables, pip); I've already done that initial work and can present it soon. But minimising changes for now seemed sensible.
ghost
mentioned this pull request
|
I'm going to merge so that I can do the other admin setting changes. Let me know if you have any issue with my two commits. |
