feat: add service info endpoint #192

JaeAeich · 2024-06-09T09:51:49Z

fixed - #157

Summary by Sourcery

This pull request adds a new service info endpoint to the TES API, providing comprehensive details about the service. It also includes significant refactoring of the TeskApp class to streamline initialization and updates the deployment configuration to support the new service info settings.

New Features:
- Introduced a new service info endpoint to provide detailed information about the TES service, including its ID, name, type, description, organization, contact URL, documentation URL, creation and update timestamps, environment, version, storage locations, and supported backend parameters.
Enhancements:
- Refactored the TeskApp class to simplify initialization by removing optional parameters and setting default values for configuration file and custom configuration model.
- Updated the deployment configuration to include custom service info settings.

JaeAeich · 2024-06-09T09:56:18Z

@sourcery-ai review

sourcery-ai · 2024-06-09T09:56:27Z

🧙 Sourcery is reviewing your pull request!

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @JaeAeich - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 4 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

sourcery-ai · 2024-06-09T09:57:23Z

tesk/api/ga4gh/tes/models/validators/rfc3339.py

@@ -0,0 +1,55 @@
+"""RFC3339 date validator."""


suggestion: Validator class naming

Consider renaming the class RFC3339 to RFC3339Validator for consistency and clarity, especially since the docstring refers to it as RFC3339Validator.

Suggested change

"""RFC3339 date validator."""

"""RFC3339Validator date validator."""

you should rename this with the purpose this validator serves or consider it in the deinition.

sourcery-ai · 2024-06-09T09:57:23Z

tesk/api/ga4gh/tes/models/base/base_tes_model.py

@@ -0,0 +1,35 @@
+"""Base class for TES API pydantic models."""


issue (bug_risk): Validator import

The import for RFC2386 should be corrected to RFC2368 to match the correct RFC for email validation.

Should we incorporate this?

sourcery-ai · 2024-06-09T09:57:23Z

tesk/api/ga4gh/tes/models/validators/rfc2386.py

@@ -0,0 +1,24 @@
+"""RFC2386 email validator."""


issue (bug_risk): Incorrect RFC number

The correct RFC for email validation is RFC 2368, not RFC 2386. Please update the class name and references accordingly.

sourcery-ai · 2024-06-09T09:57:24Z

tesk/api/ga4gh/tes/models/validators/rfc3986.py

@@ -0,0 +1,33 @@
+"""RFC3986 URL validator."""


suggestion: Validator class naming

Consider renaming the class RFC3986 to RFC3986Validator for consistency and clarity, especially since the docstring refers to it as RFC3986Validator.

Suggested change

"""RFC3986 URL validator."""

"""RFC3986Validator URL validator."""

kushagra189

Please incorporate the comments.

deployment/config.yaml

kushagra189 · 2024-06-17T04:24:32Z

tesk/api/ga4gh/tes/base/base_tesk_request.py

 	"""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:


IMO we should keep a default init.

BaseTeskRequest is implementing ABC, would we initialize super still?

deployment/config.yaml

kushagra189 · 2024-06-17T12:59:40Z

tesk/api/ga4gh/tes/controllers.py

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

+service_info = ServiceInfo()


Should we define this in the method itself?

If someone keeps hitting this endpoint, why initialise the the obj again and again, either way it is not supposed to change or be that fickel, in that case it makes more sense to give cached result.

PS: The case will be diff when/if we implement post service endpoint.

tesk/api/ga4gh/tes/controllers.py

kushagra189 · 2024-06-17T13:54:46Z

tesk/api/ga4gh/tes/models/tes_service_info_type.py

+			"a different namespace (e.g. your organization's reverse domain name)."
+		),
+	)
+	artifact: Literal['tes'] = Field(


Why Literal?

because its "Literal"ly just tes in the specs. It mentions enum there, but since there wont be anything but tes why not use literal.

kushagra189 · 2024-06-17T13:58:00Z

tesk/api/ga4gh/tes/models/validators/rfc3986.py

+			r'^(?:http|ftp)s?://'  # http:// or https://
+			r'(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+'  # domain part 1
+			r'(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|'  # domain part 2
+			r'localhost|'  # localhost...


is this correct?

I have tested with limited set of URLs and that seems to be correct afaik 😭, maybe better use the pydantic type HttpURL.

kushagra189 · 2024-06-17T13:59:08Z

tesk/api/ga4gh/tes/service_info/base/base_service_info_request.py

+from tesk.tesk_app import TeskApp
+
+
+class BaseServiceInfoRequest(BaseTeskRequest):


Do we really need this? Do you think there is a possibility of multiple input service info request?

yeah this one I don't really need this one specifically but since I will def need similar "interface" in task endpoint for consistency, I implemented interface for service as well.

tesk/api/ga4gh/tes/service_info/base/base_service_info_request.py

tesk/api/ga4gh/tes/service_info/service_info.py

JaeAeich · 2024-06-30T07:58:47Z

please check #198

JaeAeich added 3 commits June 8, 2024 16:41

index on serviceInfo: 1bf8866 ci

77ee7a3

WIP on serviceInfo: 1bf8866 ci

c00bf88

[WIP] serviceinfo

5f64301

JaeAeich changed the base branch from master to main June 9, 2024 09:51

This comment was marked as off-topic.

Sign in to view

JaeAeich changed the base branch from main to feat/baseClasses June 9, 2024 09:52

sourcery-ai bot reviewed Jun 9, 2024

View reviewed changes

JaeAeich mentioned this pull request Jun 15, 2024

Implement controllers for API operations #162

Open

5 tasks

kushagra189 requested changes Jun 17, 2024

View reviewed changes

JaeAeich added 5 commits June 19, 2024 23:42

Merge branch 'feat/baseClasses' into feat/serviceInfo

ad91ff5

merge

cb13fdb

merge

dd3daad

docs

c442cc5

minor

04740aa

JaeAeich force-pushed the feat/serviceInfo branch from 4ed93c7 to 04740aa Compare June 19, 2024 21:06

JaeAeich mentioned this pull request Jun 21, 2024

feat: add service-info endpoint #190

Closed

JaeAeich closed this Jun 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add service info endpoint #192

feat: add service info endpoint #192

JaeAeich commented Jun 9, 2024 •

edited by sourcery-ai bot

Loading

This comment was marked as off-topic.

JaeAeich commented Jun 9, 2024

sourcery-ai bot commented Jun 9, 2024

sourcery-ai bot left a comment

sourcery-ai bot Jun 9, 2024

kushagra189 Jun 17, 2024

sourcery-ai bot Jun 9, 2024

kushagra189 Jun 17, 2024

sourcery-ai bot Jun 9, 2024

sourcery-ai bot Jun 9, 2024

kushagra189 left a comment

kushagra189 Jun 17, 2024

JaeAeich Jun 19, 2024

kushagra189 Jun 17, 2024

JaeAeich Jun 19, 2024

kushagra189 Jun 17, 2024

JaeAeich Jun 19, 2024

kushagra189 Jun 17, 2024

JaeAeich Jun 19, 2024

kushagra189 Jun 17, 2024

JaeAeich Jun 19, 2024

JaeAeich commented Jun 30, 2024

	"""RFC3339 date validator."""
	"""RFC3339Validator date validator."""

		@@ -0,0 +1,35 @@
		"""Base class for TES API pydantic models."""

	"""RFC3986 URL validator."""
	"""RFC3986Validator URL validator."""

		from tesk.tesk_app import TeskApp


		class BaseServiceInfoRequest(BaseTeskRequest):

feat: add service info endpoint #192

feat: add service info endpoint #192

Conversation

JaeAeich commented Jun 9, 2024 • edited by sourcery-ai bot Loading

Summary by Sourcery

This comment was marked as off-topic.

JaeAeich commented Jun 9, 2024

sourcery-ai bot commented Jun 9, 2024

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jun 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sourcery-ai bot Jun 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sourcery-ai bot Jun 9, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jun 9, 2024

Choose a reason for hiding this comment

kushagra189 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaeAeich commented Jun 30, 2024

JaeAeich commented Jun 9, 2024 •

edited by sourcery-ai bot

Loading