feat: add service info endpoint #5

sourcery-ai-experiments-bot · 2024-07-01T02:07:49Z

Summary by Sourcery

This pull request adds a new service info endpoint to the TES API, allowing clients to retrieve metadata about the service. It includes updates to the deployment configuration to support custom service info settings and introduces a ServiceInfo class to manage the retrieval and formatting of this information.

New Features:
- Introduced a new service info endpoint for the TES API, providing metadata about the service.
Enhancements:
- Added a ServiceInfo class to handle the retrieval and formatting of service information from the configuration or default values.
Build:
- Updated deployment configuration to include custom service info settings.

sourcery-ai-experiments-bot · 2024-07-01T02:07:51Z

This is a benchmark review for experiment review_of_reviews_20240701.
Run ID: review_of_reviews_20240701/benchmark_2024-07-01T00-20-40_v1-19-0-108-g64b3508a8.

This pull request was cloned from https://github.com/elixir-cloud-aai/TESK/pull/198. (Note: the URL is not a link to avoid triggering a notification on the original pull request.)

Experiment configuration

review_config:
  # User configuration for the review
  # - benchmark - use the user config from the benchmark reviews
  # - <value> - use the value directly
  user_review_config:
    enable_ai_review: true
    enable_rule_comments: false

    enable_complexity_comments: benchmark
    enable_security_comments: benchmark
    enable_tests_comments: benchmark
    enable_comment_suggestions: benchmark
    enable_functionality_review: benchmark

    enable_pull_request_summary: benchmark
    enable_review_guide: benchmark

    enable_approvals: true

  ai_review_config:
    # The model responses to use for the experiment
    # - benchmark - use the model responses from the benchmark reviews
    # - llm - call the language model to generate responses
    model_responses:
      comments_model: benchmark
      comment_area_model: benchmark
      comment_validation_model: benchmark
      comment_suggestion_model: benchmark
      complexity_model: benchmark
      docstrings_model: benchmark
      functionality_model: benchmark
      security_model: benchmark
      tests_model: benchmark
      pull_request_summary_model: benchmark
      review_guide_model: benchmark

# The pull request dataset to run the experiment on
pull_request_dataset:
- https://github.com/henkel-v/Laravel-base-course/pull/9
- https://github.com/chraebsli/coding-crab-blog/pull/68
- https://github.com/jkool702/bash_uint/pull/2
- https://github.com/jkool702/bash_uint/pull/3
- https://github.com/NTUYWANG103/APEX_AIMBOT/pull/147
- https://github.com/dreamerminsk/tasked/pull/69
- https://github.com/dreamerminsk/tasked/pull/70
- https://github.com/dreamerminsk/tasked/pull/71
- https://github.com/dreamerminsk/tasked/pull/72
- https://github.com/dreamerminsk/tasked/pull/73
- https://github.com/dreamerminsk/tasked/pull/74
- https://github.com/LLotme/vscode-docs/pull/1
- https://github.com/gdsfactory/vtt/pull/58
- https://github.com/bengosney/cerberus/pull/957
- https://github.com/TeKrop/overfast-api/pull/149
- https://github.com/Xmaster6y/lczerolens/pull/26
- https://github.com/approvals/ApprovalTests.Dart/pull/14
- https://github.com/ShiroePL/Cooler-Ai-chan/pull/57
- https://github.com/duyet/clickhouse-monitoring/pull/277
- https://github.com/duyet/clickhouse-monitoring/pull/278
- https://github.com/duyet/charts/pull/180
- https://github.com/duyet/charts/pull/181
- https://github.com/duyet/charts/pull/182
- https://github.com/duyet/charts/pull/184
- https://github.com/duyet/charts/pull/185
- https://github.com/duyet/clickhouse-monitoring/pull/279
- https://github.com/duyet/clickhouse-monitoring/pull/280
- https://github.com/duyet/clickhouse-monitoring/pull/281
- https://github.com/duyet/charts/pull/177
- https://github.com/duyet/charts/pull/178
- https://github.com/duyet/charts/pull/179
- https://github.com/Frix-x/klippain-shaketune/pull/138
- https://github.com/mlanser/cmr-website/pull/2
- https://github.com/Jeremy-Rich/turbo-octo-spork/pull/1
- https://github.com/Quarticai/graphene-django-extras/pull/107
- https://github.com/Quarticai/QuarticSDK/pull/367
- https://github.com/Quarticai/authlib/pull/14
- https://github.com/Quarticai/djangosaml2/pull/28
- https://github.com/hbmartin/podcast-transcript-tools/pull/19
- https://github.com/ydonchoi/Study_solve_problems/pull/9
- https://github.com/jbflow/tb-backend-frameworks/pull/2
- https://github.com/jbflow/tb-backend-frameworks/pull/3
- https://github.com/elixir-cloud-aai/TESK/pull/196
- https://github.com/elixir-cloud-aai/TESK/pull/197
- https://github.com/elixir-cloud-aai/TESK/pull/198
- https://github.com/jjagielka/svelte-pivottable/pull/1
- https://github.com/MusicalNinjas/pyo3-stubgen/pull/11
- https://github.com/mrvozturk/DigitalEmporium/pull/12
- https://github.com/Shloimy15e/glicks/pull/35
- https://github.com/Shloimy15e/glicks/pull/36
review_comment_labels:
- label: correct
  question: Is this comment correct?
- label: helpful
  question: Is this comment helpful?
- label: comment-type
  question: Is the comment type correct?
- label: comment-area
  question: Is the comment area correct?
- label: llm-test
  question: |
    What type of LLM test could this comment become?
    - 👍 - this comment is really good/important and we should always make it
    - 👎 - this comment is really bad and we should never make it
    - no reaction - don't turn this comment into an LLM test

# Benchmark reviews generated by running
#   python -m scripts.experiment benchmark <experiment_name>
benchmark_reviews: []

SourceryAI · 2024-07-01T02:08:03Z

Reviewer's Guide by Sourcery

This pull request introduces a new service info endpoint to the TES API, allowing clients to retrieve metadata about the service. The implementation includes updates to the deployment configuration to support this new endpoint, changes to the TES API controllers to handle the new endpoint, and the creation of a ServiceInfo class to manage service info retrieval and response.

File-Level Changes

Files	Changes
`tesk/api/ga4gh/tes/controllers.py` `tesk/api/ga4gh/tes/service_info/service_info.py` `tesk/api/ga4gh/tes/service_info/__init__.py`	Implemented the new service info endpoint, including the ServiceInfo class and its integration into the TES API controllers.

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.

SourceryAI

Hey @sourcery-ai-experiments-bot - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

Revert type change for contactUrl (link)

Here's what I looked at during the review

🔴 General issues: 1 blocking issue, 6 other issues
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

LangSmith trace

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

SourceryAI · 2024-07-01T02:09:42Z

deployment/config.yaml

+    organization:
+      name: My organization
+      url: https://example.com
+    contactUrl: mailto:[email protected]


suggestion: Consider using a more generic contact URL

Using a mailto link directly in the configuration might limit flexibility. Consider using a more generic contact URL that can be updated without changing the configuration.

Suggested change

contactUrl: mailto:support@example.com

contactUrl: https://example.com/contact

Is this comment correct?

Is this comment helpful?

Is the comment type correct?

Is the comment area correct?

What type of LLM test could this comment become?

👍 - this comment is really good/important and we should always make it

👎 - this comment is really bad and we should never make it

no reaction - don't turn this comment into an LLM test

SourceryAI · 2024-07-01T02:09:42Z

tesk/api/ga4gh/tes/controllers.py

-                  **kwargs: Arbitrary keyword arguments.
-  """
-  pass
+def GetServiceInfo() -> dict:  # type: ignore


suggestion: Consider handling potential exceptions

The GetServiceInfo function does not handle any potential exceptions that might be raised by ServiceInfo().response(). Consider adding exception handling to provide more robust error handling.

Suggested change

def GetServiceInfo() -> dict: # type: ignore

def GetServiceInfo() -> dict: # type: ignore

try:

service_info = ServiceInfo()

except Exception as e:

return {"error": str(e)}

Is this comment correct?

Is this comment helpful?

Is the comment type correct?

Is the comment area correct?

What type of LLM test could this comment become?

👍 - this comment is really good/important and we should always make it

👎 - this comment is really bad and we should never make it

no reaction - don't turn this comment into an LLM test

SourceryAI · 2024-07-01T02:09:43Z

tesk/api/ga4gh/tes/models.py

@@ -353,7 +353,7 @@ class Service(BaseModel):
  organization: Organization = Field(
    ..., description="Organization providing the service"
  )
-  contactUrl: Optional[AnyUrl] = Field(
+  contactUrl: Optional[str] = Field(


issue (bug_risk): Revert type change for contactUrl

Changing the type of contactUrl from AnyUrl to str might lead to invalid URLs being accepted. Consider reverting this change to ensure URL validation.

Is this comment correct?

Is this comment helpful?

Is the comment type correct?

Is the comment area correct?

What type of LLM test could this comment become?

👍 - this comment is really good/important and we should always make it

👎 - this comment is really bad and we should never make it

no reaction - don't turn this comment into an LLM test

SourceryAI · 2024-07-01T02:09:43Z

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

+  def __init__(self) -> None:
+    """Initializes the BaseServiceInfoRequest class."""
+    self.service_info: Optional[TesServiceInfo] = None
+    self.config = TeskApp().conf


suggestion: Consider dependency injection for configuration

Directly instantiating TeskApp within the ServiceInfo class makes it harder to test and less flexible. Consider using dependency injection to pass the configuration.

Suggested change

self.config = TeskApp().conf

class BaseServiceInfoRequest:

def __init__(self, config=None) -> None:

"""Initializes the BaseServiceInfoRequest class."""

self.service_info: Optional[TesServiceInfo] = None

self.config = config if config is not None else TeskApp().conf

Is this comment correct?

Is this comment helpful?

Is the comment type correct?

Is the comment area correct?

What type of LLM test could this comment become?

👍 - this comment is really good/important and we should always make it

👎 - this comment is really bad and we should never make it

no reaction - don't turn this comment into an LLM test

SourceryAI · 2024-07-01T02:09:43Z

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

+      ConfigNotFoundError: If custom config or part of it is not found
+        in the config file.
+    """
+    if not self.config.custom:


suggestion (bug_risk): Check for None explicitly

Using if not self.config.custom: can be ambiguous if self.config.custom can be an empty dictionary. Consider explicitly checking for None.

Suggested change

if not self.config.custom:

if self.config.custom is None:

Is this comment correct?

Is this comment helpful?

Is the comment type correct?

Is the comment area correct?

What type of LLM test could this comment become?

👍 - this comment is really good/important and we should always make it

👎 - this comment is really bad and we should never make it

no reaction - don't turn this comment into an LLM test

SourceryAI · 2024-07-01T02:09:43Z

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

+
+    try:
+      service_info = TesServiceInfo(**service_info_data)
+    except TypeError as e:


suggestion: Consider logging the exception

Logging the exception can provide more context when debugging issues related to invalid service info format.

Suggested change

except TypeError as e:

except TypeError as e:

logging.error(f"Invalid service info format: {e}")

Is this comment correct?

Is this comment helpful?

Is the comment type correct?

Is the comment area correct?

What type of LLM test could this comment become?

👍 - this comment is really good/important and we should always make it

👎 - this comment is really bad and we should never make it

no reaction - don't turn this comment into an LLM test

SourceryAI · 2024-07-01T02:09:43Z

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

+        self.service_info = self.get_default_service_info()
+
+    # return self.service_info.dict()
+    return dict(json.loads(self.service_info.json()))


suggestion: Simplify JSON serialization

The line return dict(json.loads(self.service_info.json())) can be simplified to return self.service_info.dict() to avoid unnecessary JSON serialization and deserialization.

Suggested change

return dict(json.loads(self.service_info.json()))

return self.service_info.dict()

Is this comment correct?

Is this comment helpful?

Is the comment type correct?

Is the comment area correct?

What type of LLM test could this comment become?

👍 - this comment is really good/important and we should always make it

👎 - this comment is really bad and we should never make it

no reaction - don't turn this comment into an LLM test

JaeAeich added 2 commits June 30, 2024 13:25

wip

528f58c

serviceinfo

1028a70

SourceryAI requested changes Jul 1, 2024

View reviewed changes

	contactUrl: mailto:support@example.com
	contactUrl: https://example.com/contact

-def GetServiceInfo() -> dict:  # type: ignore
+def GetServiceInfo() -> dict:  # type: ignore
+    try:
+        service_info = ServiceInfo()
+    except Exception as e:
+        return {"error": str(e)}

	except TypeError as e:
	except TypeError as e:
	logging.error(f"Invalid service info format: {e}")

	return dict(json.loads(self.service_info.json()))
	return self.service_info.dict()

feat: add service info endpoint #5

Are you sure you want to change the base?

feat: add service info endpoint #5

Conversation

sourcery-ai-experiments-bot commented Jul 1, 2024 • edited by SourceryAI Loading

Summary by Sourcery

sourcery-ai-experiments-bot commented Jul 1, 2024

SourceryAI commented Jul 1, 2024 • edited Loading

Reviewer's Guide by Sourcery

File-Level Changes

SourceryAI 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

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

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

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

sourcery-ai-experiments-bot commented Jul 1, 2024 •

edited by SourceryAI

Loading

SourceryAI commented Jul 1, 2024 •

edited

Loading