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

1 - Implement Shodan and Credential views into report generator #191

Merged
merged 26 commits into from
Nov 3, 2022

Conversation

DJensen94
Copy link
Contributor

@DJensen94 DJensen94 commented Apr 28, 2022

fixes #172, fixes #176, fixes #181
To simplify the metrics required to generate the reports we have created new db views, this PR implements those changes across the report generator

💭 Motivation and context

A lot of the data manipulation in the metrics file is difficult to read and can be implemented into views inside our db. By querying directly from these views we speed up our report generation and simplify our code.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • These code changes follow cisagov code standards.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

To simplify the metrics required to generate the reports we have created new db views, this commit implements those changes across the report generator
@lgtm-com
Copy link

lgtm-com bot commented Apr 28, 2022

This pull request introduces 4 alerts when merging 09ac0e3 into c926100 - view on LGTM.com

new alerts:

  • 2 for Wrong name for an argument in a class instantiation
  • 1 for Variable defined multiple times
  • 1 for Redundant assignment

add server to the db_schema
@coveralls
Copy link

coveralls commented Apr 28, 2022

Coverage Status

Coverage increased (+1.8%) to 26.288% when pulling 241889f on DJ_implement_views into dd5a870 on develop.

@lgtm-com
Copy link

lgtm-com bot commented Apr 28, 2022

This pull request introduces 4 alerts when merging f78b708 into c926100 - view on LGTM.com

new alerts:

  • 2 for Wrong name for an argument in a class instantiation
  • 1 for Variable defined multiple times
  • 1 for Redundant assignment

@aloftus23 aloftus23 changed the title Implement new views into report generator 3 - Implement new views into report generator May 3, 2022
@aloftus23 aloftus23 changed the title 3 - Implement new views into report generator 3 - Implement Shodan and Credential views into report generator May 3, 2022
@aloftus23 aloftus23 changed the title 3 - Implement Shodan and Credential views into report generator 4 - Implement Shodan and Credential views into report generator May 4, 2022
@aloftus23 aloftus23 mentioned this pull request May 23, 2022
1 task
@aloftus23 aloftus23 changed the title 4 - Implement Shodan and Credential views into report generator 1 - Implement Shodan and Credential views into report generator Sep 26, 2022
@lgtm-com
Copy link

lgtm-com bot commented Sep 26, 2022

This pull request introduces 1 alert when merging 040b1de into 78ccbfd - view on LGTM.com

new alerts:

  • 1 for Redundant assignment

@lgtm-com
Copy link

lgtm-com bot commented Oct 7, 2022

This pull request introduces 1 alert when merging 65ea84f into 78ccbfd - view on LGTM.com

new alerts:

  • 1 for Redundant assignment

@aloftus23
Copy link
Contributor

@DJensen94 @cduhn17 I got this one working finally. Is it worth reviewing and setting up to be merged next? Or should we shift to another?

Copy link
Contributor Author

@DJensen94 DJensen94 left a comment

Choose a reason for hiding this comment

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

Added a couple of docstring suggestions, but other than that I think it looks good

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 and others added 3 commits October 27, 2022 14:53
Update the get_orgs function to create a connection if one is not provided
get_orgs always requires a connection so we should revert this change
@DJensen94
Copy link
Contributor Author

@dav3r This PR is ready for review

@aloftus23 aloftus23 self-assigned this Oct 31, 2022
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 all looks good to me, but I had a question and request that I'd like addressed before this gets merged.

src/pe_reports/metrics.py Outdated Show resolved Hide resolved
src/pe_reports/metrics.py Outdated Show resolved Hide resolved
@cduhn17 cduhn17 merged commit 0c89337 into develop Nov 3, 2022
@cduhn17 cduhn17 deleted the DJ_implement_views branch November 3, 2022 14:59
@aloftus23 aloftus23 moved this from PR to Done in Sprint 18 (10/31 - 11/11) Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
5 participants