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

created script to email the audit log #491

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

created script to email the audit log #491

wants to merge 6 commits into from

Conversation

BLema
Copy link

@BLema BLema commented Apr 21, 2021

Created a script for sending the day's audit log to a list of recipients created on the Castle CMS settings page.

@bduncan137
Copy link
Collaborator

@BLema , I forgot to mention that, because a registry record was added, an upgrade will be required. See upgrade 2 6 27 for the standard pattern when writing upgrade code. there are about 4 or 5 files that need to be created/editted for this

Copy link
Collaborator

@bduncan137 bduncan137 left a comment

Choose a reason for hiding this comment

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

after requested changes are made, at minimum, flake8 linter will need to be run, and code changes it recommends need to be made. Running the command flake8 castle.cms from the root directory will probably be sufficient, although you may need to pip install flake8 first. There may be errors it complains about that we don't care about. you can look at our specific flake8 config here https://github.com/castlecms/castle.cms/blob/master/setup.cfg, and you can see the automated tests that are run on every commit here: https://github.com/castlecms/castle.cms/blob/master/.travis.yml. you can also run our tests locally before committing with the command bin/test, and there are a bunch of config options that sometimes come in handy, which you can find with bin/test --help. Also, for the flake8 config, there are slightly different command-line args that can mimic the config in setup.cfg. For example, flake8 --max-line-length=110 castle.cms

@@ -464,4 +464,13 @@
handler=".upgrades.upgrade_2_6_31"
profile="castle.cms:default"
/>

Copy link
Collaborator

Choose a reason for hiding this comment

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

needs a register profile here as well

@@ -0,0 +1,8 @@
<?xml version="1.0"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file needs to be renamed

@@ -0,0 +1,4 @@
<?xml version="1.0"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file needs to be renamed


def run_query():
registry = getToolByName(app['Castle'], 'portal_registry')
# index_name = 'castle'
Copy link
Collaborator

Choose a reason for hiding this comment

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

commented lines can be removed

index_name = audit.get_index_name()
es = ESConnectionFactoryFactory(registry)()

#import pdb; pdb.set_trace()
Copy link
Collaborator

Choose a reason for hiding this comment

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

commented lines can be removed

name="2_6_32"
title="CastleCMS upgrade to 2.6.32 profile"
directory="profiles/2_6_32"
description=""
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably a good idea to throw in a short description like 2632 adds script that does something with the audit log

@@ -0,0 +1,10 @@
from Products.CMFCore.utils import getToolByName
Copy link
Collaborator

Choose a reason for hiding this comment

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

A month or so back, I refactored how we do upgrades to save some code and files. For a "standard" upgrade like this, this file is now unnecessary, but we do need a line in castle/cms/upgrades/init.py for 2_6_32.

@bduncan137
Copy link
Collaborator

if you click on details above, next to continuous-integration/travis-ci/pr or continuous-integration/travis-ci/push (red x indicates something in the ci tests failed), you should be able to see what specifically went wrong and where. In this case all 4 build jobs failed, and following the links, you can look through the build jobs, which after a bunch of boilerplate logging show ConfigurationError: ('Invalid value for', 'handler', 'ImportError: Module castle.cms.upgrades has no global upgrade_2_6_32'). This means it was unable to find an upgrade step 2_6_32, which is due to my recent refactor mentioned in another comment on the pr.

@bduncan137
Copy link
Collaborator

now just flake8 again

@bduncan137
Copy link
Collaborator

I'm still seeing 3 flake8 issues:
castle.cms/castle/cms/cron/_email_audit.py:23:30: F821 undefined name 'app'
castle.cms/castle/cms/cron/_email_audit.py:65:13: E126 continuation line over-indented for hanging indent
castle.cms/castle/cms/cron/_email_audit.py:71:9: F821 undefined name 'app'

the first and third aren't actually problems, since some magic makes 'app' available as this script is run. But flake8 doesn't know that, so we can tell flake8 to ignore the issue by putting # noqa: F821 after each of thos lines. the second just needs to have the double indent on lines 65 & 66 removed.



def run_query():
registry = getToolByName(app['Castle'], 'portal_registry') # noqa: F821
Copy link
Collaborator

Choose a reason for hiding this comment

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

in order to not create a different flake8 error, inline comments need to have exactly 2 spaces before comment

return html


def run(app): # noqa: F821
Copy link
Collaborator

Choose a reason for hiding this comment

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

this wasn't where the error was - a parameter will always be defined, even if the value is None



if __name__ == '__main__':
run(app)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this was where app was undefined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants