Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

Add centralized database query #95

Merged
merged 17 commits into from
Nov 16, 2021
Merged

Add centralized database query #95

merged 17 commits into from
Nov 16, 2021

Conversation

cduhn17
Copy link
Collaborator

@cduhn17 cduhn17 commented Nov 2, 2021 •
edited by schmelz21

🗣 Description

Centralize database query functions for pe-reports proposed in issue #77.

💭 Motivation and context

To have all database functions all in one place for logical organization.

🧪 Testing

The following processes must run successfully
Install: pip install -e .
Tests: pytest /tests
Additionally pass all pre-commits
pre-commit run --all-files

📷 Screenshots (if appropriate)

✅ Checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All future TODOs are captured in issues, which are referenced
    in code comments.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

@cduhn17 cduhn17 added improvement This issue or pull request will add or improve functionality, maintainability, or ease of use upstream update This issue or pull request pulls in upstream updates labels Nov 2, 2021
@cduhn17 cduhn17 self-assigned this Nov 2, 2021
@coveralls
Copy link

coveralls commented Nov 2, 2021 •
edited

Pull Request Test Coverage Report for Build 1464277222

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 76.987%
Totals Coverage Status
Change from base Build 1436493065: 0.0%
Covered Lines: 174
Relevant Lines: 217
💛 - Coveralls

@schmelz21 schmelz21 linked an issue Nov 2, 2021 that may be closed by this pull request
Create scripts to pull data from Posture & Exposure database #77
Closed
8 tasks
@cduhn17 cduhn17 marked this pull request as ready for review November 3, 2021 20:37
@schmelz21 schmelz21 self-requested a review November 4, 2021 10:22
schmelz21
schmelz21 suggested changes Nov 4, 2021
View reviewed changes
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
aloftus23
aloftus23 reviewed Nov 4, 2021
View reviewed changes
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
cduhn17 and others added 2 commits November 4, 2021 14:18
@cduhn17 @aloftus23
Update logging message
5f73916 
Co-authored-by: aloftus23 <79927030+aloftus23@users.noreply.github.com>
Update logging errors
323382c
aloftus23
aloftus23 approved these changes Nov 4, 2021
View reviewed changes
Copy link
Contributor

@aloftus23 aloftus23 left a comment

Choose a reason for hiding this comment

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

Looks good.

schmelz21
schmelz21 approved these changes Nov 4, 2021
View reviewed changes
Copy link
Collaborator

@schmelz21 schmelz21 left a comment

Choose a reason for hiding this comment

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

Looks good.

@schmelz21
Copy link
Collaborator

@dav3r or @jsf9k, do you want to take a look before we merge?

@dav3r
Copy link
Member

dav3r commented Nov 5, 2021

@dav3r or @jsf9k, do you want to take a look before we merge?

I would like to take a look at this, but I am not sure if I will be able to get to it today.

jsf9k
jsf9k reviewed Nov 8, 2021
View reviewed changes
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

I made one suggestion.

src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
@cduhn17 @jsf9k
Update logging message error
e5e2403 
Co-authored-by: Shane Frasier <jeremy.frasier@trio.dhs.gov>
dav3r
dav3r suggested changes Nov 10, 2021
View reviewed changes
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

👍 on centralizing DB queries, but please take a look at my questions and comments.

src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Outdated Show resolved Hide resolved
src/pe_reports/data/db_query.py Show resolved Hide resolved
@cduhn17 @dav3r
Update doc string to be more exact
58f02a9 
Co-authored-by: dav3r <david.redmin@trio.dhs.gov>
@cduhn17 cduhn17 requested a review from dav3r November 12, 2021 15:43
dav3r
dav3r approved these changes Nov 15, 2021
View reviewed changes
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

This has my 👍 as soon as my final item is addressed.

@cduhn17 cduhn17 merged commit 8463c48 into develop Nov 16, 2021
@cduhn17 cduhn17 deleted the CD-wip-db-query branch November 16, 2021 13:17
@cduhn17 cduhn17 moved this from PR to DONE in Sprint 6 (10/15 - 12/1) Nov 16, 2021
cisagovbot pushed a commit that referenced this pull request Jan 11, 2022
@mcdonnnj
Merge pull request #95 from cisagov/improvement/update_ansible-lint_c…
11eca90 
…onfiguration

Update `ansible-lint` Configuration
aloftus23 pushed a commit that referenced this pull request Apr 13, 2022
@cduhn17
Merge pull request #95 from cisagov/CD-wip-db-query
469e27d 
Add centralized database query
cisagovbot pushed a commit that referenced this pull request May 5, 2022
@mcdonnnj
Merge pull request #95 from cisagov/improvement/update_build_process
7dc735b 
Modernize the `build` step in the `build` workflow
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dav dav dav left review comments

@jsf9k jsf9k jsf9k left review comments

@dav3r dav3r dav3r approved these changes

@aloftus23 aloftus23 aloftus23 approved these changes

@schmelz21 schmelz21 schmelz21 approved these changes

@DJensen94 DJensen94 Awaiting requested review from DJensen94 DJensen94 is a code owner

@felddy felddy Awaiting requested review from felddy felddy is a code owner

@mcdonnnj mcdonnnj Awaiting requested review from mcdonnnj mcdonnnj is a code owner

@stewartl97 stewartl97 Awaiting requested review from stewartl97 stewartl97 is a code owner

Assignees

@cduhn17 cduhn17

Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use upstream update This issue or pull request pulls in upstream updates
Projects
No open projects
Sprint 6 (10/15 - 12/1)
DONE
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create scripts to pull data from Posture & Exposure database
7 participants
@cduhn17 @coveralls @schmelz21 @dav3r @dav @jsf9k @aloftus23