cal-itp / eligibility-server Goto Github PK
View Code? Open in Web Editor NEWServer implementation of the Eligibility Verification API
Home Page: https://docs.calitp.org/eligibility-server
License: GNU Affero General Public License v3.0
Server implementation of the Eligibility Verification API
Home Page: https://docs.calitp.org/eligibility-server
License: GNU Affero General Public License v3.0
See Flask - Handling Application Errors
Specifically, we should look at Returning API Errors as JSON.
403
(forbidden, not authenticated/authorized)404
(endpoint not found)500
(other application error / Exception)from @thekaveman in #30
The current Database
class is a simple wrapper around the server.json
file.
The public API check_user(key, user, types)
method is great from a consumer point of view. But the internals are tied too closely to the server.json
file structure.
The database may proxy to a number of data sources:
server.json
like file with nested data.csv
file of users in a flat tableWe should refactor to allow different use-cases, perhaps through inheritance or some plug-in mechanism.
Supporting a request of the server's public key directly may help simplify the benefits app.
These tests are where you might want to do some of the environment monkeypatching to ensure settings variables get the correct values from the environment etc.
Originally posted by @thekaveman in #51 (comment)
Dockerfile
, devcontainer.json
, docker-compose.yml
, bin/pre-commit
, .env.sample
into a new top-level directory, /.devcontainer
Why? To align this repository's folder architecture with benefits, eligibility-api and current best practices.
We should configure logging for the project as a whole and add relevant statements throughout the code.
Maybe a follow-up: figure out how to get the Python
logging
framework configured and replace the
Originally posted by @thekaveman in #104 (review)
The current Docker image (via bin/start.sh
) starts the Flask development server.
This has been fine as we've only been using the server for testing. As we move to deploy the server for Courtesy Cards, we'll want to run Flask using production best-practices.
See more at: Flask - Deploying to Production.
In general, we can follow a similar pattern as in benefits
:
nginx
is the reverse proxy that accepts traffic coming into the container, and routes app traffic alonggunicorn
is the WSGI application server, receiving app traffic from nginx
and forwarding to the app (Flask)bin/start.sh
starts the production setupWe want the main
branch to be protected:
git push --force
(we should merge all changes via PR)pre-commit
check to passSettings here: https://github.com/cal-itp/eligibility-server/settings/branches
So we can automate review requests, etc.
We currently pin conventional-pre-commit
to v1.0.0
There have been some recent syntax and support improvements on that hook. A new tagging/versioning mechanism means v1
will always point to the latest v1.x.x
tag.
Let's pin to v1
so we always get the latest.
The docker-compose.yml over in benefits
defines the server
service and uses a local build path when the image doesn't yet exist on the machine:
server:
build: ./server #<-- build the contents of the local ./server directory (which contains a Dockerfile)
image: eligibility_verification_server:dev
Once the server code is refactored out of that repository and into this one, a local build path won't work anymore.
GitHub Container Registry can be used to host Docker images, which can be built and published automatically via GitHub Actions.
Outstanding bugs:
From the main directory, run coverage run -m pytests
-> should be coverage run -m pytest
localhost/server
codeI am unable to get pre-commit
running in my local devcontainer. This is the error log I get:
pre-commit version: 2.19.0
git --version: git version 2.30.2
sys.version:
3.9.13 (main, May 18 2022, 04:17:41)
[GCC 10.2.1 20210110]
sys.executable: /usr/local/bin/python
os.name: posix
sys.platform: linux
An unexpected error has occurred: CalledProcessError: command: ('/root/.cache/pre-commit/repofu2cxgzn/py_env-python3.9/bin/python', '-mpip', 'install', '.')
return code: 1
expected return code: 0
stdout:
Processing /root/.cache/pre-commit/repofu2cxgzn
Preparing metadata (setup.py): started
Preparing metadata (setup.py): finished with status 'done'
Collecting ruamel.yaml>=0.15
Using cached ruamel.yaml-0.17.21-py3-none-any.whl (109 kB)
Collecting toml
Using cached toml-0.10.2-py2.py3-none-any.whl (16 kB)
Collecting ruamel.yaml.clib>=0.2.6
Using cached ruamel.yaml.clib-0.2.6.tar.gz (180 kB)
Preparing metadata (setup.py): started
Preparing metadata (setup.py): finished with status 'error'
stderr:
error: subprocess-exited-with-error
× python setup.py egg_info did not run successfully.
│ exit code: 1
╰─> [3 lines of output]
sys.argv ['/tmp/pip-install-jxph4ctq/ruamel-yaml-clib_c3f62e16a11e4867877ec98577ee7f67/setup.py', 'egg_info', '--egg-base', '/tmp/pip-pip-egg-info-bdkc5a4p']
test compiling /tmp/tmp_ruamel_n_ndon63/test_ruamel_yaml.c -> test_ruamel_yaml compile error: /tmp/tmp_ruamel_n_ndon63/test_ruamel_yaml.c
Exception: command 'gcc' failed: No such file or directory
[end of output]
note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed
× Encountered error while generating package metadata.
╰─> See above for output.
note: This is an issue with the package mentioned above, not pip.
hint: See above for details.
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/pre_commit/error_handler.py", line 73, in error_handler
yield
File "/usr/local/lib/python3.9/site-packages/pre_commit/main.py", line 389, in main
return run(args.config, store, args)
File "/usr/local/lib/python3.9/site-packages/pre_commit/commands/run.py", line 414, in run
install_hook_envs(to_install, store)
File "/usr/local/lib/python3.9/site-packages/pre_commit/repository.py", line 223, in install_hook_envs
_hook_install(hook)
File "/usr/local/lib/python3.9/site-packages/pre_commit/repository.py", line 79, in _hook_install
lang.install_environment(
File "/usr/local/lib/python3.9/site-packages/pre_commit/languages/python.py", line 221, in install_environment
helpers.run_setup_cmd(prefix, install_cmd)
File "/usr/local/lib/python3.9/site-packages/pre_commit/languages/helpers.py", line 51, in run_setup_cmd
cmd_output_b(*cmd, cwd=prefix.prefix_dir, **kwargs)
File "/usr/local/lib/python3.9/site-packages/pre_commit/util.py", line 146, in cmd_output_b
raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)
pre_commit.util.CalledProcessError: command: ('/root/.cache/pre-commit/repofu2cxgzn/py_env-python3.9/bin/python', '-mpip', 'install', '.')
return code: 1
expected return code: 0
stdout:
Processing /root/.cache/pre-commit/repofu2cxgzn
Preparing metadata (setup.py): started
Preparing metadata (setup.py): finished with status 'done'
Collecting ruamel.yaml>=0.15
Using cached ruamel.yaml-0.17.21-py3-none-any.whl (109 kB)
Collecting toml
Using cached toml-0.10.2-py2.py3-none-any.whl (16 kB)
Collecting ruamel.yaml.clib>=0.2.6
Using cached ruamel.yaml.clib-0.2.6.tar.gz (180 kB)
Preparing metadata (setup.py): started
Preparing metadata (setup.py): finished with status 'error'
stderr:
error: subprocess-exited-with-error
× python setup.py egg_info did not run successfully.
│ exit code: 1
╰─> [3 lines of output]
sys.argv ['/tmp/pip-install-jxph4ctq/ruamel-yaml-clib_c3f62e16a11e4867877ec98577ee7f67/setup.py', 'egg_info', '--egg-base', '/tmp/pip-pip-egg-info-bdkc5a4p']
test compiling /tmp/tmp_ruamel_n_ndon63/test_ruamel_yaml.c -> test_ruamel_yaml compile error: /tmp/tmp_ruamel_n_ndon63/test_ruamel_yaml.c
Exception: command 'gcc' failed: No such file or directory
[end of output]
note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed
× Encountered error while generating package metadata.
╰─> See above for output.
note: This is an issue with the package mentioned above, not pip.
hint: See above for details.
We have a pretty good devcontainer setup in cal-itp/benefits
in terms of the extensions installed and default settings.
Let's replicate all that over here to make them more consistent.
The Database
class (and server.json
) currently contain configuration data, separate from the user data needed for verification. This configuration should be split out as a separate concern.
These were added when this was part of the benefits
project to help testing other API integrations outside of the core Eligibility Verification API.
Remove:
/static/
folder and contentsMerchantAuthToken
Database.check_merchant
and related codeserver.json
: config.request_access
and merchants
dataInstead of getting config manually from the settings
module like:
app = Flask(__name__)
app.name = settings.APP_NAME
app.config["SQLALCHEMY_DATABASE_URI"] = settings.SQLALCHEMY_DATABASE_URI
# etc.
Flask has built-in support for loading the configuration from a number of sources. We're using the Python files format, which would look something like:
app = Flask(__name__)
app.config.from_object("eligibility_server.settings")
We can also load from a number of supported file types:
import json
app.config.from_file("config.json", load=json.load)
Or directly from certain environment variables:
app.config.from_prefixed_env()
Let's use the built-in framework for loading configuration data.
See also: Flask Configuration Best Practices
On startup, the server
container should import user data as specified by the IMPORT_FILE_PATH
environment variable.
As noted in cal-itp/benefits#468, the server
container as defined by our Docker Compose configuration currently starts the Flask application but does not initialize any data.
This is because the code in Database
expects User
models to already have been imported.
https://github.com/cal-itp/eligibility-server/blob/main/eligibility_server/verify.py#L114
This line should be configurable
if re.match(r"^[A-Z]\d{7}$", sub):
We might want to configure this default level
to be something different depending on the environment (local, dev, production, etc.)
What do you think about moving the entire logging configuration into the settings.py
module and adding an environment variable similar to benefits
?
Originally posted by @thekaveman in #116 (comment)
app.py
is one giant file with a lot going on. As this project starts to mature, it makes sense to refactor the existing file into a few different modules that can grow and be maintained a little more independently.
Wait until #25 is closed to make it easier!
pytest
, coverage
dependenciesBuilding on #30 and cal-itp/hashfields
, we want to support (at least) two distinct modes of verification lookup from our source:
(id, name)
exist on the list?(hash(id), hash(name))
exist on the list?(note in either case, the Eligibility Request coming into the server will contain the un-hashed data)
We'll want to be able to configure:
sha256
, sha384
, sha512
, others?)We currently use Python 3.9 as defined in a number of places including Docker images and GitHub Actions settings.
Let's update to the more recent version 3.10.
Add CodeQL GitHub Action for Eligibility Server
GH_PROJECT
(the project number)Docs Deploy GitHub Action should only run when files in this list are changed:
https://github.com/cal-itp/eligibility-server/blob/main/.github/workflows/mkdocs.yml#L7-L10
As we move into becoming production-ready, there are a few items of cleanup in the Database
class that should happen.
cc @angela-tran who discussed this with me earlier today and informed this ticket.
cc @machikoyasuda we held off on some of this to make #116 more straightforward, documenting here instead.
Method | Feedback |
---|---|
__init__() |
let's not query and store all users. We may have a large list of users, and don't necessarily want to hold them all in-memory |
__init__() |
log the given hash option as DEBUG |
check_user() |
break up large if(...) statement checking the various states into if/elif/else etc. |
check_user() |
query for the user here |
check_user() |
separate calculation of the return value from the return statement |
check_user() |
add various logging to the above refactors |
Let's also verify the tests are providing adequate coverage and scoping.
This corresponds to cal-itp/hashfields#18
The app.py
module currently sits at the root of the repository. This is fine for a single file, but can cause trouble when trying to import objects from the module (see e.g. #10) or when the code becomes more complicated than a single file.
Recommendation is to create a subdirectory, named eligibility_server
, and place both the app.py
and an (empty) __init__.py
file in there.
Elsewhere (e.g. in tests) you could then:
from eligibility_server import app
This is the corollary to #100, but for the devcontainer.
This will make the experience of getting the application running just a little faster by pre-initializing the database.
.devcontainer/pre-commit.sh
to .devcontainer/postAttach.sh
(also update the devcontainer.json
)python setup.py
to the post attach scriptThe pre-commit
Action is failing here too, e.g. run 2060074713
First we'll need to login to pyup.io as the cal-itp-bot account and link the repo. We can pair on this piece.
This will generate and commit a .pyup.yml
file in the repo, and we'll probably need to adjust to our needs. See the config from benefits: https://github.com/cal-itp/benefits/blob/dev/.pyup.yml
--
Use Dependabot instead and do research
This is a corollary to cal-itp/benefits#141, with which this work must coordinate. See that issue for the general overview.
eligibility_server/verify.py
is where the server-side implementation lives.
Some of the code there is setting up the flask_restful
resource and dealing with HTTP request processing, and that would stay. Anything concerned with creating and processing the Eligibility Request and Response objects could be moved into the new package:
The eligibility server needs to be hosted somewhere such that it can receive and respond to HTTP requests from a Benefits client instance.
The owner of the hosting environment is not as important as are the agreement(s) in place that allow for sharing of (hashed) Courtesy Card data with said hosting environment.
The main decision point for hosting of the Eligibility Server:
Will the server be co-located with the Benefits client app deployment (CDT/Azure), or will the server be deployed to a new environment?
The server would just be another Azure App Service.
See the original writeup in cal-itp/benefits#123 and the implementation in cal-itp/benefits#124.
Relevant docs: https://docs.calitp.org/benefits/getting-started/docker-dynamic-ports/
We should apply the same treatment here.
To the latest version possible.
We should make the CSV and JSON data test files match the schema we need for future releases
Currently when setting up from a CSV input file, some settings are assumed, but we likely want them to be configurable:
if settings.IMPORT_FILE_FORMAT == "csv":
with open(settings.IMPORT_FILE_PATH, newline="", encoding="utf-8") as file:
data = csv.reader(file, delimiter=";", quotechar="", quoting=csv.QUOTE_NONE)
delimiter
may not be a semicolon ;
Let's make these settings that can be configured from the environment.
Currently, an attempt to restart a server
Docker container will result in an IntegrityError
:
+ python setup.py
Creating table...
Table created.
Importing users from /.devcontainer/server/data.csv
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1819, in _execute_context
self.dialect.do_execute(
File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
cursor.execute(statement, parameters)
sqlite3.IntegrityError: UNIQUE constraint failed: user.key
A separate but similar issue is that an attempt to teardown the database if it has already been torn down results in an OperationalError
:
+ python teardown.py
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1819, in _execute_context
self.dialect.do_execute(
File "/usr/local/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
cursor.execute(statement, parameters)
sqlite3.OperationalError: no such table: user
We should make the setup
and teardown
scripts idempotent to prevent these errors.
#101 made it so the app container initializes the database on startup.
#99 introduced an environment variable to configure the expected format for the sub
field from requests.
The docs should be updated to include this setting.
Tracking a todo in this repository based on a number of related issues:
We currently push the server image tagged with main
to GHCR for every commit to the main
branch, and tagged with the commit SHA and latest
for every Release.
As we're seeing elsewhere, a more structured version/release strategy could help bring clarity to the codebase. We should pick a strategy here that aligns across the Benefits portfolio (as in the linked issues).
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.