-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
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 left a couple of comments for your consideration.
More generally, I just wanted to confirm that it makes sense to include this WAS data in the P&E database. If you are ok tying their data to yours (and all of the access and maintenance ramifications that come with that), then it's fine. I am only making sure that you have thought about how this might affect things a year or two (or longer!) down the road.
dav3r reviewed 3 hours ago setup.py "pymupdf", "python-dateutil >= 2.7.3", "pytest-cov", "python-pptx == 0.6.21", "pytz", "pyyaml == 6.0", "reportlab == 3.6.6", "reportlab", Member @dav3r dav3r 3 hours ago The PR description should mention that you are removing these version pins and why you are doing so. @cduhn17 Reply... Merge state Add more commits by pushing to the CD-pe-reports-add-WAS-schema branch on cisagov/pe-reports. Code owner review required Waiting on code owner review from DJensen94, aloftus23, dav3r, felddy, jsf9k, mcdonnnj, and/or stewartl97. 6 pending reviewers View Unresolved conversations 3 conversations must be resolved before merging. Some checks were not successful 10 failing, 3 successful, 4 skipped, and 4 expected checks @github-actions build / lint (pull_request) Failing after 2m Required Details @github-actions build / lint (push) Failing after 2m Required Details @github-actions CodeQL / Analyze (python) (pull_request) Successful in 22m Required Details @github-actions CodeQL / Analyze (python) (push) Successful in 22m Required Details @github-actions build / test (3.7) (pull_request) Failing after 48s Required Details @github-actions build / test (3.7) (push) Failing after 1m Required Details @github-actions build / test (3.8) (pull_request) Failing after 48s Required Details @github-actions build / test (3.8) (push) Failing after 49s Required Details @github-actions build / test (3.9) (pull_request) Failing after 49s Required Details @github-actions build / test (3.9) (push) Failing after 55s Required Details @github-actions build / test (3.10) (pull_request) Failing after 2m Required Details @github-actions build / test (3.10) (push) Failing after 2m Required Details @github-actions build / coveralls-finish (pull_request) Skipped Required Details @github-actions build / coveralls-finish (push) Skipped Required Details @github-actions build / build (pull_request) Skipped Details @github-actions build / build (push) Skipped Details build (3.10) Expected β Waiting for status to be reported Required build (3.7) Expected β Waiting for status to be reported Required build (3.8) Expected β Waiting for status to be reported Required build (3.9) Expected β Waiting for status to be reported Required @github-code-scanning Code scanning results / CodeQL Successful in 4s β No new or fixed alerts Required Details Merging is blocked Merging can be performed automatically with 2 approving reviews. You can also open this in GitHub Desktop or view . @cduhn17 Add heading textAdd bold text, <Cmd+b>Add italic text, <Cmd+i> Add a quote, <Cmd+Shift+.>Add code, <Cmd+e>Add a link, <Cmd+k> Add a bulleted list, <Cmd+Shift+8>Add a numbered list, <Cmd+Shift+7>Add a task list, <Cmd+Shift+l> Directly mention a user or team Reference an issue, pull request, or discussion Add saved reply Leave a comment No file chosen Attach files by dragging & dropping, selecting or pasting them. Styling with Markdown is supported Remember, contributions to this repository should follow its contributing guidelines and security policy. ProTip! Add .patch or .diff to the end of URLs for Gitβs plaintext views. Reviewers @dav3r dav3r dav3r left review comments Re-request review @aloftus23 aloftus23 @DJensen94 DJensen94 @felddy felddy @jsf9k jsf9k @mcdonnnj mcdonnnj @stewartl97 stewartl97 At least 2 approving reviews are required to merge this pull request. Still in progress? Assignees @cduhn17 cduhn17 Labels Draft PR PR in draft mode, not passing checks Projects Sprint 20 PR Milestone No milestone Development Successfully merging this pull request may close these issues. None yet Notifications Customize Youβre receiving notifications because you were assigned. 3 participants @cduhn17 @coveralls @dav3r Revert "Update package versions" This reverts commit 7253164.
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.
Looks good
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 PR description also mentions vw_cidrs
, but I don't see anything in this PR about a table or schema with that name.
Co-authored-by: dav3r <david.redmin@trio.dhs.gov>
@dav3r , I removed the reference to |
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.
LGTM π
π£ Description
Adding table to database schema:
was_tracker_customerdata
π Motivation and context
The WAS team will use P&E database instance to house the data for reports that are run by the WAS team. Additionally, this table will be available via P&E UI and REST API endpoint.
β Pre-approval checklist
in code comments.
to reflect the changes in this PR.