-
Notifications
You must be signed in to change notification settings - Fork 3
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
3106 reparse django action #3167
base: develop
Are you sure you want to change the base?
Conversation
…ub.com:raft-tech/TANF-app into 3076-Admin-Filter-Enhancements-Data-Files-Page
…ub.com:raft-tech/TANF-app into 3076-Admin-Filter-Enhancements-Data-Files-Page
…F-app into 3106-reparse-django-action
…F-app into 3106-reparse-django-action
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.
This generally works, but i'm adding the comments below for discussion.
def get_actions(self, request): | ||
"""Return the actions.""" | ||
actions = super().get_actions(request) | ||
if not request.user.groups.filter(name__in=["OFA System Admin", "OFA Admin"]).exists(): |
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.
user.is_in_group()
or user.is_ofa_sys_admin
(modified to accept both)
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.
The problem is then it will do two queries instead of one
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.
is it possible to rename this?
|
||
|
||
disableFields = function() { | ||
console.log('disabling fields'); |
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.
should this function do something else?
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.
this was for testing purposes
from django.core.management import call_command | ||
from django.utils.translation import ngettext | ||
from django.contrib import messages | ||
call_command("clean_and_reparse", f"-f {file_ids}") |
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.
one reason for this - i was testing the sequential reparse scenario and was confronted with this error
i couldn't find the actual exception in the logs (the parse was still active, and putting out tons of logs), but if we re-implemented the command logic, that error could be caught and displayed to the user
$(window).on('load', function() { | ||
//your code here | ||
console.log('loaded'); | ||
var S=document.querySelector('button[type=submit]'); // add the first listener |
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.
can we name this submitBtn, or something a little more verbose?
|
||
js = ('admin/js/admin/mymodel.js',) | ||
|
||
actions = ['reparse_cmd'] |
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.
nit: can we rename this to just reparse
?
I tried executing the action with ~1k files to see what would happen. The files weren't associated to any records, I just wanted to see if 1k IDs would serialize over to the command. I either got the Nginx "an error occurred" page or the stack trace below. Not sure why the DB wasn't backing up under this condition. INFO Starting clean and reparse command for FY All and Q1-4
2024-09-27 09:11:45 2024-09-27 13:11:45,335 INFO clean_and_reparse.py::__backup:L48 : Beginning reparse DB Backup.
2024-09-27 09:11:45 INFO Beginning reparse DB Backup.
2024-09-27 09:11:45 sh: 1: pg_dump: not found
2024-09-27 09:11:45 ERROR Database backup FAILED. Clean and reparse NOT executed. Database and Elastic are CONSISTENT!
2024-09-27 09:11:45 ERROR Internal Server Error: /admin/data_files/datafile/
2024-09-27 09:11:45 Traceback (most recent call last):
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py", line 47, in inner
2024-09-27 09:11:45 response = get_response(request)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/core/handlers/base.py", line 181, in _get_response
2024-09-27 09:11:45 response = wrapped_callback(request, *callback_args, **callback_kwargs)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 616, in wrapper
2024-09-27 09:11:45 return self.admin_site.admin_view(view)(*args, **kwargs)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
2024-09-27 09:11:45 response = view_func(request, *args, **kwargs)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
2024-09-27 09:11:45 response = view_func(request, *args, **kwargs)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/sites.py", line 232, in inner
2024-09-27 09:11:45 return view(request, *args, **kwargs)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 43, in _wrapper
2024-09-27 09:11:45 return bound_method(*args, **kwargs)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
2024-09-27 09:11:45 response = view_func(request, *args, **kwargs)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 1723, in changelist_view
2024-09-27 09:11:45 response = self.response_action(request, queryset=cl.get_queryset(request))
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 1408, in response_action
2024-09-27 09:11:45 response = func(self, request, queryset)
2024-09-27 09:11:45 File "/tdpapp/tdpservice/data_files/admin/admin.py", line 44, in reparse_cmd
2024-09-27 09:11:45 call_command("clean_and_reparse", f"-f {file_ids}")
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 181, in call_command
2024-09-27 09:11:45 return command.execute(*args, **defaults)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 398, in execute
2024-09-27 09:11:45 output = self.handle(*args, **options)
2024-09-27 09:11:45 File "/tdpapp/tdpservice/search_indexes/management/commands/clean_and_reparse.py", line 328, in handle
2024-09-27 09:11:45 self.__backup(backup_file_name, log_context)
2024-09-27 09:11:45 File "/tdpapp/tdpservice/search_indexes/management/commands/clean_and_reparse.py", line 59, in __backup
2024-09-27 09:11:45 raise e
2024-09-27 09:11:45 File "/tdpapp/tdpservice/search_indexes/management/commands/clean_and_reparse.py", line 49, in __backup
2024-09-27 09:11:45 call_command('backup_db', '-b', '-f', f'{backup_file_name}')
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 181, in call_command
2024-09-27 09:11:45 return command.execute(*args, **defaults)
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 398, in execute
2024-09-27 09:11:45 output = self.handle(*args, **options)
2024-09-27 09:11:45 File "/tdpapp/tdpservice/scheduling/management/commands/backup_db.py", line 53, in handle
2024-09-27 09:11:45 raise e
2024-09-27 09:11:45 File "/tdpapp/tdpservice/scheduling/management/commands/backup_db.py", line 48, in handle
2024-09-27 09:11:45 if os.path.getsize(file) == 0:
2024-09-27 09:11:45 File "/usr/local/lib/python3.10/genericpath.py", line 50, in getsize
2024-09-27 09:11:45 return os.stat(filename).st_size
2024-09-27 09:11:45 FileNotFoundError: [Errno 2] No such file or directory: '/tmp/reparsing_backup_selected_files_rpv3.pg' |
It looks like it is not finding PD_DUMP command, might be because of docker problem. Will take a look again |
@raftmsohani you're absolutely right. I forgot that wasnt being pulled into the container! |
actions = super().get_actions(request) | ||
if not request.user.groups.filter(name__in=["OFA System Admin", "OFA Admin"]).exists(): | ||
if "reparse" in actions: | ||
del actions["reparse"] |
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.
Instead of another if
statement can we just make it actions.pop("reparse", None)
?
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.
Unfortunately no since actions is a list and .pop only works with int indexes
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.
If actions
is a list how are you indexing into it with a string?
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.
you are right, it is a dictionary and although my original test failed, I tested again and it passed. so will do the change
After my pg_client blunder I retested what happens with the command when large amounts of files are selected. I tested with 3000 files and every single file ID made it to the command and was reparsed. So I think in general we should be good on that! I did face one issue though. When executing the command I was not presented with the confirmation dialog before being redirected to the ReparseMeta page. |
@raftmsohani Follow on. I cleared my cache and re-tried all 3000 files. When I do that I get the following error and cannot submit the action at all. Looks like there is still a reference to that debug function you had. |
Removing the missing function from the JS, clearing cache, and re-executing action with 3000 files does present the confirmation dialog, however it has the wrong number of files listed. |
} | ||
submitBtn.addEventListener('click', function(e) { | ||
e.preventDefault(); | ||
disableFields(); |
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.
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.
should be fixed now
|
Summary of Changes
Pull request closes #3106
How to Test
Deliverables
More details on how deliverables herein are assessed included here.
Acceptance Criteria
Deliverable 1: Accepted Features
Checklist of ACs:
lfrohlich
and/oradpennington
confirmed that ACs are met.Deliverable 2: Tested Code
CodeCov Report
comment in PR)CodeCov Report
comment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjolly
andttran-hub
using Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):