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

feat: add service-info endpoint #190

Closed
wants to merge 6 commits into from
Closed
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
4 changes: 2 additions & 2 deletions .github/workflows/code_quality.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ jobs:
with:
os: ${{ job.os }}
python-version: '3.11'
poetry-install-options: "--only=types --no-root"
poetry-export-options: "--only=types"
poetry-install-options: "--only=main --only=types --no-root"
poetry-export-options: "--only=main --only=types"

- name: Check types
run: poetry run mypy tesk/
Expand Down
25 changes: 25 additions & 0 deletions deployment/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,31 @@ api:
swagger_ui: True
serve_spec: True

# ServiceInfo configuration based on ServiceInfo model
serviceInfo:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding this in the custom section. Then you could also provide a module with Pydantic models and have FOCA validate these config params. I think this is done in proTES and maybe some other services that use a recent FOCA version.

id: org.ga4gh.myservice
name: My project
type:
group: org.ga4gh
artifact: tes
version: 1.0.0
description: This service provides...
organization:
name: My organization
url: https://example.com
contactUrl: mailto:[email protected]
documentationUrl: https://docs.myservice.example.com
createdAt: '2019-06-04T12:58:19Z'
updatedAt: '2019-06-04T12:58:19Z'
environment: test
version: 1.0.0
storage:
- file:///path/to/local/funnel-storage
- s3://ohsu-compbio-funnel/storage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also put a dummy value here

tesResources_backend_parameters:
- VmSize
- ParamToRecogniseDataComingFromConfig

# Logging configuration
# Cf. https://foca.readthedocs.io/en/latest/modules/foca.models.html#foca.models.config.LogConfig
log:
Expand Down
6 changes: 6 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Global options:

[mypy]
plugins = pydantic.mypy
warn_return_any = True
warn_unused_configs = True

Expand All @@ -12,3 +13,8 @@ ignore_missing_imports = True

[mypy-foca]
ignore_missing_imports = True

[pydantic-mypy]
init_forbid_extra = True
init_typed = True
warn_required_dynamic_aliases = True
13 changes: 12 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ kubernetes = "^29.0.0"
python = "^3.11"
requests = ">=2.20.0"
urllib3 = "^2.2.1"
types-pyyaml = "^6.0.12.20240311"

[tool.poetry.group.docs.dependencies]
added-value = "^0.24.0"
Expand Down Expand Up @@ -68,6 +69,7 @@ types-botocore = "^1.0.2"
types-requests = "^2.31.0.20240406"
types-urllib3 = "^1.26.25.14"
types-werkzeug = "^1.0.9"
types-pyyaml = "^6.0.12.20240311"

[tool.poetry.scripts]
api = 'tesk.app:main'
Expand Down
6 changes: 6 additions & 0 deletions tesk/api/ga4gh/tes/base/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""Base classes for the TES API.

This module contains the base abstract class which is needed by all the
subclasses of the TES API, serving as single source of truth for the common
business logic, properties and methods.
"""
46 changes: 46 additions & 0 deletions tesk/api/ga4gh/tes/base/base_tesk_request.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
"""Base classes for the TES API request."""

from abc import ABC, abstractmethod
from typing import Any, final

from pydantic import BaseModel

from tesk.tesk_app import TeskApp


class BaseTeskRequest(ABC, TeskApp):
"""Base class for the TES API.

This class is an abstract class that defines the common properties and
methods needed by all of the TES API endpoint business logic.
"""

def __init__(self) -> None:
"""Initializes the BaseTeskRequest class."""
super().__init__()

@abstractmethod
def api_response(self) -> BaseModel:
"""Returns the response as Pydantic model.

Should be implemented by the child class as final
business logic for the specific endpoint.

Returns:
BaseModel: API response for the specific endpoint.
"""
pass

@final
def response(self) -> dict[Any, Any]:
"""Returns serialized response.

Should be invoked by controller.

Returns:
dict: Serialized response for the specific endpoint.
"""
_response = self.api_response()
if not isinstance(_response, BaseModel):
raise TypeError('API response must be a Pydantic model.')
return _response.dict()
15 changes: 7 additions & 8 deletions tesk/api/ga4gh/tes/controllers.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
"""Controllers for GA4GH TES API endpoints."""

import logging
from typing import Any

# from connexion import request # type: ignore
from foca.utils.logging import log_traffic # type: ignore

from tesk.api.ga4gh.tes.service_info.service_info import ServiceInfo

# Get logger instance
logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -36,14 +39,10 @@ def CreateTask(*args, **kwargs) -> dict: # type: ignore

# GET /tasks/service-info
@log_traffic
def GetServiceInfo(*args, **kwargs) -> dict: # type: ignore
"""Get service info.

Args:
*args: Variable length argument list.
**kwargs: Arbitrary keyword arguments.
"""
pass
def GetServiceInfo() -> dict[Any, Any]:
"""Get service info."""
service_info = ServiceInfo()
return service_info.response()


# GET /tasks
Expand Down
1 change: 1 addition & 0 deletions tesk/api/ga4gh/tes/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""TESK API GA4GH TES models."""
195 changes: 195 additions & 0 deletions tesk/api/ga4gh/tes/models/tes_service_info.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
"""TesServiceInfo model, used to represent the service information."""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Consider using consistent naming conventions.

The attribute created_at uses snake_case, while updatedAt uses camelCase. For consistency, it would be better to use one naming convention throughout the model.

Suggested change
"""TesServiceInfo model, used to represent the service information."""
"""TesServiceInfo model, used to represent the service information."""
import logging
class TesServiceInfo:
def __init__(self, created_at, updated_at):
self.created_at = created_at
self.updated_at = updated_at

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uniqueg please look at this, why are there naming inconsistency in the openAPI? Is there a specific reason (can we raise a PR regarding this rn and fix it, maybe not huh 🤔 ) because this is not just some places and errs alot, I had to check the name in the yaml again and again 😢.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there lots of inconsistencies. I hate it, too. The problem is that naming changes are generally breaking changes, so while they're easy to fix, they are very consequential changes.

But if you like, you could suggest these changes for improved consistency, but keep the old versions and add a deprecation note saying that the inconsistently named props will be removed with the next major version release. You would have my support at least.


import logging
from contextlib import suppress
from typing import List, Optional

from pydantic import BaseModel, Field, ValidationError, validator

from tesk.api.ga4gh.tes.models.tes_service_info_organization import (
TesServiceInfoOrganization,
)
from tesk.api.ga4gh.tes.models.tes_service_info_type import TesServiceInfoType
from tesk.api.ga4gh.tes.models.validators.rfc2386_validator import RFC2386Validator
from tesk.api.ga4gh.tes.models.validators.rfc3339_validator import RFC3339Validator
from tesk.api.ga4gh.tes.models.validators.rfc3986_validator import RFC3986Validator

logger = logging.getLogger(__name__)


class TesServiceInfo(BaseModel):
"""TesServiceInfo model, used to represent the service information.

Attributes:
id (str): Unique ID of this service.
name (str): Name of this service.
type (TesServiceInfoType): Type of a GA4GH service.
description (str): Description of the service.
organization (TesServiceInfoOrganization): Organization providing the service.
contactUrl (str): URL of the contact for the provider of this service.
documentationUrl (str): URL of the documentation for this service.
created_at (str): Timestamp describing when the service was first deployed
and available.
updatedAt (str): Timestamp describing when the service was last updated.
environment (str): Environment the service is running in.
version (str): Version of the service being described.
storage (List[str]): Lists some, but not necessarily all, storage locations
supported by the service.
tesResources_backend_parameters (List[str]): Lists of supported
tesResources.backend_parameters
keys.

Example:
{
"id": "org.ga4gh.myservice",
"name": "My project",
"type": {
"group": "org.ga4gh",
"artifact": "tes",
"version": "1.0.0"
},
"description": "This service provides...",
"organization": {
"name": "My organization",
"url": "https://example.com"
},
"contactUrl": "mailto:[email protected]",
"documentationUrl": "https://docs.myservice.example.com",
"createdAt": "2019-06-04T12:58:19Z",
"updatedAt": "2019-06-04T12:58:19Z",
"environment": "test",
"version": "1.0.0",
"storage": [
"file:///path/to/local/funnel-storage",
"s3://ohsu-compbio-funnel/storage"
]
}
"""

id: str = Field(
...,
example='org.ga4gh.myservice',
description=(
'Unique ID of this service. Reverse domain name notation is recommended, '
'though not required. The identifier should attempt to be globally unique '
'so it can be used in downstream aggregator services e.g. Service Registry.'
),
)
name: str = Field(
...,
example='My project',
description='Name of this service. Should be human readable.',
)
type: TesServiceInfoType
description: Optional[str] = Field(
default=None,
example='This service provides...',
description=(
'Description of the service. Should be human readable and '
'provide information about the service.'
),
)
organization: TesServiceInfoOrganization
contactUrl: Optional[str] = Field(
default=None,
example='mailto:[email protected]',
description=(
'URL of the contact for the provider of this service, '
'e.g. a link to a contact form (RFC 3986 format), '
'or an email (RFC 2368 format).'
),
)
documentationUrl: Optional[str] = Field(
default=None,
example='https://docs.myservice.example.com',
description='URL of the documentation for this service.',
)
created_at: Optional[str] = Field(
default=None,
example='2019-06-04T12:58:19Z',
description=(
'Timestamp describing when the service was first deployed '
'and available (RFC 3339 format)'
),
)
updatedAt: Optional[str] = Field(
default=None,
example='2019-06-04T12:58:19Z',
description=(
'Timestamp describing when the service was last updated (RFC 3339 format)'
),
)
environment: Optional[str] = Field(
default=None,
example='test',
description=(
'Environment the service is running in. Use this to distinguish '
'between production, development and testing/staging deployments. '
'Suggested values are prod, test, dev, staging. However this is '
'advised and not enforced.'
),
)
version: str = Field(
...,
example='1.0.0',
description=(
'Version of the service being described. Semantic versioning is '
'recommended, but other identifiers, such as dates or commit hashes, '
'are also allowed. The version should be changed whenever the service '
'is updated.'
),
)
storage: List[str] = Field(
default_factory=list,
example=[
'file:///path/to/local/funnel-storage',
's3://ohsu-compbio-funnel/storage',
],
description=(
'Lists some, but not necessarily all, storage locations supported '
'by the service.'
),
)
tesResources_backend_parameters: List[str] = Field(
default_factory=list,
example=['VmSize'],
description=(
'Lists all tesResources.backend_parameters keys supported by the service'
),
)

# Remarks:
# @uniqueg: There is a better way to do this, create a ValidateClass, which
# has all the validators and date sanitizers, create a
# BaseTeskModel(BaseModel, ValidateClass), this class will then be implemented
# by all the models, and the validators will be reused.
# The issue with this approach is that we don't have consistent field names and I
# feel they might be subject to change in future or among different models.
# This is why I have not implemented this approach.
#
# Another cool approach would be to create a decorator, I tried that but given
# FOCA uses pydantic v1.*, and if FOCA is to be upgraded to v2.*, the decorator
# validate would be removed and the code would break.

# validators
_ = validator('documentationUrl', allow_reuse=True)(RFC3986Validator().validate)
_ = validator('created_at', 'updatedAt', allow_reuse=True)(
RFC3339Validator().validate
)

@validator('contactUrl')
def validate_url_and_email(cls, v: str) -> str:
"""Validate the contactURL format based on RFC 3986 or 2368 standard."""
url_validator = RFC3986Validator()
email_validator = RFC2386Validator()

with suppress(ValidationError):
return email_validator.validate(cls, v)

with suppress(ValidationError):
return url_validator.validate(cls, v)

logger.error('contactUrl must be based on RFC 3986 or 2368 standard.')
raise ValidationError(
'contactUrl must be based on RFC 3986 or 2368 standard.', model=cls
)
Loading
Loading