Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch-API: avoid committing on GET endpoints #378

Merged
merged 1 commit into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions jobbergate-api/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ This file keeps track of all notable changes to jobbergate-api

Unreleased
----------
- Changed internals to avoid committing to the database when a GET request is made
- Added extra settings to allow profiling and tracing on sentry

4.1.0a2 -- 2023-10-10
---------------------
Expand Down
2 changes: 2 additions & 0 deletions jobbergate-api/jobbergate_api/apps/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def service_factory(session: AsyncSession, bucket: Bucket) -> Iterator[Services]
def secure_services(
*scopes: str,
permission_mode: PermissionMode = PermissionMode.ALL,
commit: bool = True,
ensure_email: bool = False,
ensure_client_id: bool = False,
):
Expand All @@ -131,6 +132,7 @@ async def dependency(
secure_session(
*scopes,
permission_mode=permission_mode,
commit=commit,
ensure_email=ensure_email,
ensure_organization=settings.MULTI_TENANCY_ENABLED is True,
ensure_client_id=ensure_client_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ async def job_script_template_create(
)
async def job_script_template_get(
id_or_identifier: int | str = Path(),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW, commit=False)),
):
"""Get a job script template by id or identifier."""
logger.info(f"Getting job script template with {id_or_identifier=}")
Expand All @@ -77,7 +77,7 @@ async def job_script_template_get(
async def job_script_template_get_list(
list_params: ListParams = Depends(),
include_null_identifier: bool = Query(False),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW, commit=False)),
):
"""Get a list of job script templates."""
logger.debug("Preparing to list job script templates")
Expand Down Expand Up @@ -143,7 +143,7 @@ async def job_script_template_delete(
async def job_script_template_get_file(
id_or_identifier: int | str = Path(),
file_name: str = Path(),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW, commit=False)),
):
"""
Get a job script template file by id or identifier.
Expand Down Expand Up @@ -221,7 +221,7 @@ async def job_script_template_delete_file(
)
async def job_script_workflow_get_file(
id_or_identifier: int | str = Path(),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_TEMPLATES_VIEW, commit=False)),
):
"""
Get a workflow file by id or identifier.
Expand Down
6 changes: 3 additions & 3 deletions jobbergate-api/jobbergate_api/apps/job_scripts/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ async def job_script_create_from_template(
)
async def job_script_get(
id: int = Path(),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SCRIPTS_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SCRIPTS_VIEW, commit=False)),
):
"""Get a job script by id."""
logger.info(f"Getting job script {id=}")
Expand All @@ -164,7 +164,7 @@ async def job_script_get_list(
None,
description="Filter job-scripts by the job-script-template-id they were created from.",
),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SCRIPTS_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SCRIPTS_VIEW, commit=False)),
):
"""Get a list of job scripts."""
logger.debug("Preparing to list job scripts")
Expand Down Expand Up @@ -227,7 +227,7 @@ async def job_script_delete(
async def job_script_get_file(
id: int = Path(...),
file_name: str = Path(...),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SCRIPTS_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SCRIPTS_VIEW, commit=False)),
):
"""
Get a job script file.
Expand Down
8 changes: 4 additions & 4 deletions jobbergate-api/jobbergate_api/apps/job_submissions/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ async def job_submission_create(
)
async def job_submission_get(
id: int = Path(...),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SUBMISSIONS_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SUBMISSIONS_VIEW, commit=False)),
):
"""Return the job_submission given it's id."""
logger.debug(f"Getting job submission {id=}")
Expand Down Expand Up @@ -119,7 +119,7 @@ async def job_submission_get_list(
None,
description="Filter job-submissions by the job-script-id they were created from.",
),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SUBMISSIONS_VIEW)),
secure_services: SecureService = Depends(secure_services(Permissions.JOB_SUBMISSIONS_VIEW, commit=False)),
):
"""List job_submissions for the authenticated user."""
logger.debug("Fetching job submissions")
Expand Down Expand Up @@ -236,7 +236,7 @@ async def job_submission_agent_update(
)
async def job_submissions_agent_pending(
secure_services: SecureService = Depends(
secure_services(Permissions.JOB_SUBMISSIONS_VIEW, ensure_client_id=True)
secure_services(Permissions.JOB_SUBMISSIONS_VIEW, commit=False, ensure_client_id=True)
),
):
"""Get a list of pending job submissions for the cluster-agent."""
Expand All @@ -262,7 +262,7 @@ async def job_submissions_agent_pending(
)
async def job_submissions_agent_active(
secure_services: SecureService = Depends(
secure_services(Permissions.JOB_SUBMISSIONS_VIEW, ensure_client_id=True)
secure_services(Permissions.JOB_SUBMISSIONS_VIEW, commit=False, ensure_client_id=True)
),
):
"""Get a list of active job submissions for the cluster-agent."""
Expand Down
2 changes: 2 additions & 0 deletions jobbergate-api/jobbergate_api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class Settings(BaseSettings):
# Sentry configuration
SENTRY_DSN: Optional[HttpUrl]
SENTRY_SAMPLE_RATE: float = Field(1.0, gt=0.0, le=1.0)
SENTRY_PROFILING_SAMPLE_RATE: float = Field(1.0, gt=0.0, le=1.0)
SENTRY_TRACING_SAMPLE_RATE: float = Field(1.0, gt=0.0, le=1.0)

# Maximum number of bytes allowed for file uploads
MAX_UPLOAD_FILE_SIZE: int = 100 * 1024 * 1024 # 100 MB
Expand Down
5 changes: 3 additions & 2 deletions jobbergate-api/jobbergate_api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
Main file to startup the fastapi server.
"""
import sys
import typing
from contextlib import asynccontextmanager

import asyncpg
Expand Down Expand Up @@ -47,8 +46,10 @@
logger.info("Initializing Sentry")
sentry_sdk.init(
dsn=settings.SENTRY_DSN,
sample_rate=typing.cast(float, settings.SENTRY_SAMPLE_RATE), # The cast silences mypy
sample_rate=settings.SENTRY_SAMPLE_RATE,
environment=settings.DEPLOY_ENV,
profiles_sample_rate=settings.SENTRY_PROFILING_SAMPLE_RATE,
traces_sample_rate=settings.SENTRY_TRACING_SAMPLE_RATE,
)
subapp.add_middleware(SentryAsgiMiddleware)
else:
Expand Down
3 changes: 2 additions & 1 deletion jobbergate-api/jobbergate_api/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class SecureSession:
def secure_session(
*scopes: str,
permission_mode: PermissionMode = PermissionMode.ALL,
commit: bool = True,
ensure_email: bool = False,
ensure_organization: bool = False,
ensure_client_id: bool = False,
Expand Down Expand Up @@ -171,7 +172,7 @@ async def dependency(
if is_test:
logger.debug("Committing nested transaction due to test mode")
await nested_transaction.commit()
else:
elif commit is True:
logger.debug("Committing session")
await session.commit()
except Exception as err:
Expand Down