-
Notifications
You must be signed in to change notification settings - Fork 4
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
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
…F-app into 3106-reparse-django-action
@ADPennington did the change on log message. Please re-review |
@raftmsohani when you have some time, Can you get the merge conflicts resolved? |
tdrs-backend/tdpservice/search_indexes/management/commands/clean_and_reparse.py
Show resolved
Hide resolved
tdrs-backend/tdpservice/search_indexes/management/commands/clean_and_reparse.py
Show resolved
Hide resolved
logger_context=log_context, | ||
level='info') | ||
else: | ||
log(f"Starting clean and reparse action for files: {str(selected_files)}", |
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.
@jtimpe I am confirming that this log change works as expected when triggering reparsing from DAC and terminal on a specific files 👍🏾
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.
@@ -33,6 +33,7 @@ def add_arguments(self, parser): | |||
parser.add_argument("-y", "--fiscal_year", type=int, help="Reparse all files in the fiscal year, e.g. 2021.") | |||
parser.add_argument("-a", "--all", action='store_true', help="Clean and reparse all datafiles. If selected, " | |||
"fiscal_year/quarter aren't necessary.") | |||
parser.add_argument("-f", "--files", nargs='+', type=str, help="Re-parse specific datafiles by datafile id") |
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.
i tested this new option on a subset of files with this command:
python manage.py clean_and_reparse -f "1215,1030,2246,1807,1737,1826,1044"
and this immediately triggered the reparsing process and did not prompt me to confirm that I wanted to reparse X number of files.
vcap@:~$ python manage.py clean_and_reparse -f "1215,1030,2246,1807,1737,1826,1044"
************** reparse all False
************** selected files [1215, 1030, 2246, 1807, 1737, 1826, 1044]
INSIDE FILE COUNTS MATCH:
7, 7, 0
INSIDE FILE COUNTS MATCH:
7, 7, 0
2024-10-17 22:01:51,087 INFO clean_and_reparse.py::_backup:L49 : Beginning reparse DB Backup.
Beginning reparse DB Backup.
2024-10-17 22:01:51,092 INFO db_backup.py::get_system_values:L54 : Using postgres client at: /home/vcap/deps/0/apt/usr/lib/postgresql/15/bin/
Using postgres client at: /home/vcap/deps/0/apt/usr/lib/postgresql/15/bin/
2024-10-17 22:01:51,094 INFO db_backup.py::backup_database:L89 : Executing backup command: /home/vcap/deps/0/apt/usr/lib/postgresql/15/bin/pg_dump -Fc
...
...
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.
i expected something like this:
vcap@:~$ python manage.py clean_and_reparse -y 2025
************** reparse all False
************** selected files None
You have selected to reparse datafiles for FY 2025 and Q1-4. The reparsed files will NOT be stored in new indices and the old indices
These options will delete and reparse (2) datafiles.
Continue [y/n]?
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.
per Jan:
-f is only used by the action (which confirms via javascript), which is why the additional conditional checks weren't added
evidence here
} | ||
submitBtn.addEventListener('click', function(e) { | ||
e.preventDefault(); | ||
if (confirm("You are about to re-parse " + number_of_files.innerHTML.split(/(\s+)/)[0] + " files. Are you sure you want to continue?")) { |
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.
Excellent work @raftmsohani 🚀 testing notes below ⬇️ cc: @ttran-hub
Tests
-
via DAC:
- most recent version of all files submitted for FY21, tribes only.
- one file
- selected list of files
- while one reparsing process was in-progress, triggering another reparse action
-
via management command (terminal):
- all FY2025 files
- selected list of files (by file id)
Notes
-
All of the tests were successful 🚀
-
Tests reaffirmed that its possible to trigger a new parsing action while one is in-progress, but it seems better to wait until the initial process is complete (the metamodel id for the new action will not appear until the initial process completes).
-
the LogEntries currently includes a list of files by ID to be reparsed, but could use some improvement to its more clear which files have been queued for reparsing. Relatedly, it would be helpful to add a logentry and/or email notification to admins when the reparsing process finished. In the meantime, we should monitor the metamodel metrics.
-
The following fields in the reparse metamodel are N/A, especially when reparsing from DAC, and may need to be refactored or removed in Re-parse command refactor #3205
-
Reparsing via terminal seems most useful if we want to reparse all versions of files by fiscal period. It is technically possible to select specific files to be reparsed, but confirmation prompts aren't currently included before the process begins, so not recommended at this time (reference)
⚠️
Summary of Changes
Pull request closes #3106
How to Test
Deliverables
More details on how deliverables herein are assessed included here.
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):