First commits for Base Application Logic #2

nickviola · 2022-02-03T21:12:14Z

🗣 Description

This covers the initial commits for the Li-PCA Web Application project and base framework setup.

💭 Motivation and context

Getting the Li-PCA Web application foundation in place for testing and further development.

🧪 Testing

Run docker compose up -d or pip install -e . && python3 -m api (see README) in the root of the repo to run this project and test.

✅ Pre-approval 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.

✅ Pre-merge checklist

Revert dependencies to default branches.
Finalize version.

✅ Post-merge checklist

Add a tag or create a release.

coveralls · 2022-02-03T21:16:16Z

Pull Request Test Coverage Report for Build 2748793141

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-100.0%) to 0.0%

Totals
Change from base Build 2277381765:	-100.0%
Covered Lines:	0
Relevant Lines:	0

💛 - Coveralls

…mmit errors

dav3r

I started reviewing, but I have to stop for now. Please feel free to let me know if you have questions about anything that I commented on. I will try to come back to this tomorrow to review the remaining files.

Also, you probably want to remove the following files from source control:

src/api/.Dockerfile.swp
src/api/openapi/.openapi.yaml.swp

And I suspect that you also no longer need src/api/data/secret.txt, but I wasn't 100% sure about that one.

CONTRIBUTING.md

dav3r · 2022-07-25T19:11:38Z

.gitignore

+*.DS_store
+*.vscode


Please put these into their own sections and alphabetize the sections, e.g.

## IDE - VS Code ## .vscode ## MacOS ## .DS_store ## Python ## ...

CONTRIBUTING.md

dav3r · 2022-07-25T19:18:03Z

CONTRIBUTING.md

-### Running unit and system tests ###
-
-In addition to the pre-commit checks the CI system will run the suite
-of unit and system tests that are included with this project.  To run
-these tests locally execute `pytest` from the root of the project.
-
-We encourage any updates to these tests to improve the overall code
-coverage.  If your pull request adds new functionality we would
-appreciate it if you extend existing test cases, or add new ones to
-exercise the newly added code.
-


Why did you delete this block of text?

This must have gotten removed unintentionally. I'll get it added back.

src/api/models/base_model_.py

src/api/models/customer.py

dav3r · 2022-07-25T21:17:30Z

src/api/models/customer.py

+from datetime import date, datetime  # noqa: F401
+from typing import Dict, List  # noqa: F401


Why are you importing these if you are not using them?

src/api/models/customer.py

dav3r · 2022-07-25T21:19:40Z

src/api/models/customer.py

+        :param uuid: The uuid of this Customer.  # noqa: E501
+        :type uuid: str
+        :param name: The name of this Customer.  # noqa: E501
+        :type name: str
+        :param contact: The contact of this Customer.  # noqa: E501
+        :type contact: str
+        :param status: The status of this Customer.  # noqa: E501
+        :type status: str
+        """
+        # noqa: E501
+        self.swagger_types = {
+            "uuid": str,
+            "name": str,
+            "contact": str,
+            "status": str,
+        }  # type: ignore


More unnecessary E501 and type: ignores that should be taken care of here and below.

addressed in b0be2dc

The E501s are taken care of here, but I'm going to leave this conversation open until the type: ignores have also been sorted out.

Sounds good. I'm working on the cleanup now and will reference the commit here once it's pushed up. There shouldn't be a need for any linter exceptions, honestly. We should address the root issues now to keep things clean.