cachito
cachito copied to clipboard
Initial connexion implementation
[DONE] Adding the connexion library and configuring cachito-api to use connexion in app.py [DONE] Routing the API endpoints to their respective cachito-api functions [WIP] Updating unit tests as necessary
Maintainers will complete the following section
- [ ] Commit messages are descriptive enough
- [ ] Code coverage from testing does not decrease and new code is covered
- [ ] New code has type annotations
- [ ] OpenAPI schema is updated (if applicable)
- [ ] DB schema change has corresponding DB migration (if applicable)
- [ ] README updated (if worker configuration changed, or if applicable)
This pull request fixes 3 alerts when merging c4dfe8ca3f57254dc53dd7d8d2ee1cdc907096f8 into dc80ba57867e578cf073dba6b0a10e5d8ac79f1b - view on LGTM.com
fixed alerts:
- 3 for Uncontrolled data used in path expression
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.
I'm stuck on the part about fixing the broken unit tests.
I started trying to fix the test: tests/test_api_v1.py::test_create_and_fetch_request
,
I got this result (getting a 404 from the client).
I tried adding the following fixture to conftest.py, using this stackoverflow answer as a base (with the necessary changes related to our code), but I kept getting the same error.
flask_app = connexion.FlaskApp(__name__)
flask_app.add_api('swagger.yml')
@pytest.fixture(scope='module')
def client():
with flask_app.app.test_client() as c:
yield c
Any suggestion? :D
I'm stuck on the part about fixing the broken unit tests.
I started trying to fix the test:
tests/test_api_v1.py::test_create_and_fetch_request
,I got this result (getting a 404 from the client).
I tried adding the following fixture to conftest.py, using this stackoverflow answer as a base (with the necessary changes related to our code), but I kept getting the same error.
flask_app = connexion.FlaskApp(__name__) flask_app.add_api('swagger.yml') @pytest.fixture(scope='module') def client(): with flask_app.app.test_client() as c: yield c
Any suggestion? :D
The following tests made the test pass locally:
diff --git a/cachito/web/api_v1.py b/cachito/web/api_v1.py
index d840d86..0c51b55 100644
--- a/cachito/web/api_v1.py
+++ b/cachito/web/api_v1.py
@@ -153,16 +153,16 @@ def get_requests():
return flask.jsonify(response)
-def get_request(request_id):
+def get_request(id):
"""
Retrieve details for the given request.
- :param int request_id: the value of the request ID
+ :param int id: the value of the request ID
:return: a Flask JSON response
:rtype: flask.Response
:raise NotFound: if the request is not found
"""
- json = Request.query.get_or_404(request_id).to_json()
+ json = Request.query.get_or_404(id).to_json()
if json["state"] == RequestStateMapping.complete.name:
package_count = len(json["packages"])
@@ -171,7 +171,7 @@ def get_request(request_id):
flask.current_app.logger.info(
"Returning data for request %i. Found %i packages and %i dependencies. "
"The following package managers were used: %s.",
- request_id,
+ id,
package_count,
dependency_count,
json["pkg_managers"],
diff --git a/cachito/web/models.py b/cachito/web/models.py
index 8c0523a..7058608 100644
--- a/cachito/web/models.py
+++ b/cachito/web/models.py
@@ -439,13 +439,13 @@ class Request(db.Model):
if verbose:
rv["configuration_files"] = flask.url_for(
- "api_v1.get_request_config_files", request_id=self.id, _external=True
+ "/api/v1.cachito_web_api_v1_get_request_config_files", id=self.id, _external=True
)
rv["content_manifest"] = flask.url_for(
- "api_v1.get_request_content_manifest", request_id=self.id, _external=True
+ "/api/v1.cachito_web_api_v1_get_request_content_manifest", id=self.id, _external=True
)
rv["environment_variables_info"] = flask.url_for(
- "api_v1.get_request_environment_variables", request_id=self.id, _external=True
+ "/api/v1.cachito_web_api_v1_get_request_environment_variables", id=self.id, _external=True
)
# Use this list comprehension instead of a RequestState.to_json method to avoid
# including redundant information about the request itself
@@ -469,7 +469,7 @@ class Request(db.Model):
if flask.current_app.config["CACHITO_REQUEST_FILE_LOGS_DIR"]:
rv["logs"] = {
"url": flask.url_for(
- "api_v1.get_request_logs", request_id=self.id, _external=True
+ "/api/v1.cachito_web_api_v1_get_request_logs", id=self.id, _external=True
)
}
else:
This pull request fixes 3 alerts when merging efdbed069d8236644fadb3344c539d0a2c9bbc64 into dc80ba57867e578cf073dba6b0a10e5d8ac79f1b - view on LGTM.com
fixed alerts:
- 3 for Uncontrolled data used in path expression
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.
This pull request fixes 3 alerts when merging 1e95b68e97a91be40bb83c2610e618fd8070d448 into dc80ba57867e578cf073dba6b0a10e5d8ac79f1b - view on LGTM.com
fixed alerts:
- 3 for Uncontrolled data used in path expression
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.
This pull request fixes 3 alerts when merging 1d4c5c4748462dceff8336c636e484e3f78736a0 into dc80ba57867e578cf073dba6b0a10e5d8ac79f1b - view on LGTM.com
fixed alerts:
- 3 for Uncontrolled data used in path expression
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.
This pull request fixes 3 alerts when merging 731e0c6317277d7eae5f60abc693f617ae3fa165 into dc80ba57867e578cf073dba6b0a10e5d8ac79f1b - view on LGTM.com
fixed alerts:
- 3 for Uncontrolled data used in path expression
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.
This pull request fixes 3 alerts when merging 2e7c3197566edf283c2d37b01bb9b9fa7b8e8fd4 into dc80ba57867e578cf073dba6b0a10e5d8ac79f1b - view on LGTM.com
fixed alerts:
- 3 for Uncontrolled data used in path expression
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.
This pull request fixes 3 alerts when merging c31b9972091624eb5009768d47aac265207ca91f into dc80ba57867e578cf073dba6b0a10e5d8ac79f1b - view on LGTM.com
fixed alerts:
- 3 for Uncontrolled data used in path expression
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed :rocket:. For more information, please check out our post on the GitHub blog.
Can we squash commits here, please?
I personally don't see much value into separating each fix for the unit tests into a single commit. I'd squash all FIX commits here.
But that's me, let's see what others think.
Do we still need this? I believe we removed all of the other references to it. https://github.com/containerbuildsystem/cachito/blob/f626d97ea669e12879a72762e03aa92d862d66a6/cachito/web/static/api_v1.yaml#L1270-L1273