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

Replace Get_orgs #640

Merged
merged 12 commits into from
Jul 25, 2023
Merged

Replace Get_orgs #640

merged 12 commits into from
Jul 25, 2023

Conversation

edujosemena
Copy link
Contributor

@edujosemena edujosemena commented Jul 18, 2023

Fixes #639

🗣 Description

For preventing sql injection errors the team is focusing on creating more API endpoints to interact with instead of using direct API calls. To get this going we need to modify the get_orgs function in db_query to get the data from the api endpoint in crossfeed rather than directly use TSQL.

Implementation notes

  • Replace get_orgs()
  • Assure the same data is received with using the api rather than TSQL
  • Get URL and API_KEY in database.ini file

🧪 Testing

API endpoint is successfully called using the requests library

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • 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.

@edujosemena edujosemena linked an issue Jul 18, 2023 that may be closed by this pull request
2 tasks
@coveralls
Copy link

coveralls commented Jul 18, 2023

Coverage Status

coverage: 26.737% (+0.1%) from 26.628% when pulling 1713f77 on EM-get-org-api into 818ecce on develop.

@dav3r
Copy link
Member

dav3r commented Jul 21, 2023

@edujosemena You should ensure that you have done all of the items in the "Pre-approval checklist" (and checked the checkboxes) before requesting approval of this PR. You may delete any that don't apply, e.g. if you have no TODO items, you may remove the "All future TODOs are captured in issues, which are referenced in code comments." item.

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.

This looks OK to me, but please consider my one suggestion.

src/pe_source/data/pe_db/db_query_source.py Outdated Show resolved Hide resolved
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.

Looks good to me aside from one minor thing I mentioned.

src/pe_reports/data/database.ini Outdated Show resolved Hide resolved
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.

Requesting correction of one documentation oversight.

src/pe_source/data/pe_db/db_query_source.py Show resolved Hide resolved
@jsf9k jsf9k added documentation This issue or pull request improves or adds to documentation improvement This issue or pull request will add or improve functionality, maintainability, or ease of use python Pull requests that update Python code labels Jul 25, 2023
@cduhn17 cduhn17 merged commit 998fa20 into develop Jul 25, 2023
25 checks passed
@cduhn17 cduhn17 deleted the EM-get-org-api branch July 25, 2023 17:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation This issue or pull request improves or adds to documentation improvement This issue or pull request will add or improve functionality, maintainability, or ease of use python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update get_orgs to use API endpoint
7 participants