-
Notifications
You must be signed in to change notification settings - Fork 13
Make the ProcessingService endpoint_url nullable #1090
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Generated by Django 4.2.10 on 2026-01-16 17:36 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("ml", "0025_alter_algorithm_task_type"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AlterField( | ||
| model_name="processingservice", | ||
| name="endpoint_url", | ||
| field=models.CharField(blank=True, max_length=1024, null=True), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ class ProcessingService(BaseModel): | |
| name = models.CharField(max_length=255) | ||
| description = models.TextField(blank=True) | ||
| projects = models.ManyToManyField("main.Project", related_name="processing_services", blank=True) | ||
| endpoint_url = models.CharField(max_length=1024) | ||
| endpoint_url = models.CharField(max_length=1024, null=True, blank=True) | ||
| pipelines = models.ManyToManyField("ml.Pipeline", related_name="processing_services", blank=True) | ||
| last_checked = models.DateTimeField(null=True) | ||
| last_checked_live = models.BooleanField(null=True) | ||
|
|
@@ -48,7 +48,8 @@ class ProcessingService(BaseModel): | |
| objects = ProcessingServiceManager() | ||
|
|
||
| def __str__(self): | ||
| return f'#{self.pk} "{self.name}" at {self.endpoint_url}' | ||
| endpoint_display = self.endpoint_url or "No endpoint configured" | ||
| return f'#{self.pk} "{self.name}" at {endpoint_display}' | ||
|
|
||
| class Meta: | ||
| verbose_name = "Processing Service" | ||
|
|
@@ -151,6 +152,19 @@ def get_status(self, timeout=90) -> ProcessingServiceStatusResponse: | |
| Args: | ||
| timeout: Request timeout in seconds per attempt (default: 90s for serverless cold starts) | ||
| """ | ||
| # If no endpoint URL is configured, return a no-op response | ||
| if self.endpoint_url is None: | ||
| return ProcessingServiceStatusResponse( | ||
| timestamp=datetime.datetime.now(), | ||
| request_successful=False, | ||
| server_live=None, | ||
| pipelines_online=[], | ||
| pipeline_configs=[], | ||
| endpoint_url=self.endpoint_url, | ||
| error="No endpoint URL configured - service operates in pull mode", | ||
| latency=0.0, | ||
| ) | ||
|
Comment on lines
+155
to
+166
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Treat blank endpoint_url as missing in status/config fetches. Because 💡 Suggested fix- if self.endpoint_url is None:
+ if not self.endpoint_url:
return ProcessingServiceStatusResponse(
timestamp=datetime.datetime.now(),
request_successful=False,
server_live=None,
pipelines_online=[],
pipeline_configs=[],
endpoint_url=self.endpoint_url,
error="No endpoint URL configured - service operates in pull mode",
latency=0.0,
)- if self.endpoint_url is None:
+ if not self.endpoint_url:
return []Also applies to: 232-234 🤖 Prompt for AI Agents |
||
|
|
||
| ready_check_url = urljoin(self.endpoint_url, "readyz") | ||
| start_time = time.time() | ||
| error = None | ||
|
|
@@ -215,6 +229,9 @@ def get_pipeline_configs(self, timeout=6): | |
| Get the pipeline configurations from the processing service. | ||
| This can be a long response as it includes the full category map for each algorithm. | ||
| """ | ||
| if self.endpoint_url is None: | ||
| return [] | ||
|
|
||
| info_url = urljoin(self.endpoint_url, "info") | ||
| resp = requests.get(info_url, timeout=timeout) | ||
| resp.raise_for_status() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,6 +109,9 @@ def check_processing_services_online(): | |
| services = ProcessingService.objects.all() | ||
|
|
||
| for service in services: | ||
| if service.endpoint_url is None: | ||
| logger.warning(f"Processing service {service} has no endpoint URL, skipping.") | ||
| continue | ||
|
Comment on lines
+112
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle blank endpoint_url as missing too.
💡 Suggested fix- if service.endpoint_url is None:
+ if not service.endpoint_url:
logger.warning(f"Processing service {service} has no endpoint URL, skipping.")
continue🤖 Prompt for AI Agents |
||
| logger.info(f"Checking service {service}") | ||
| try: | ||
| status_response = service.get_status() | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -115,6 +115,51 @@ def test_processing_service_pipeline_registration(self): | |||||
|
|
||||||
| self.assertEqual(pipelines_queryset.count(), len(response["pipelines"])) | ||||||
|
|
||||||
| def test_create_processing_service_without_endpoint_url(self): | ||||||
| """Test creating a ProcessingService without endpoint_url (pull mode)""" | ||||||
| processing_services_create_url = reverse_with_params("api:processingservice-list") | ||||||
| self.client.force_authenticate(user=self.user) | ||||||
| processing_service_data = { | ||||||
| "project": self.project.pk, | ||||||
| "name": "Pull Mode Service", | ||||||
| "description": "Service without endpoint", | ||||||
| } | ||||||
| resp = self.client.post(processing_services_create_url, processing_service_data) | ||||||
| self.client.force_authenticate(user=None) | ||||||
|
|
||||||
| self.assertEqual(resp.status_code, 201) | ||||||
| data = resp.json() | ||||||
|
|
||||||
| # Check that endpoint_url is null | ||||||
| self.assertIsNone(data["instance"]["endpoint_url"]) | ||||||
|
|
||||||
| # Check that status indicates no endpoint configured | ||||||
| self.assertFalse(data["status"]["request_successful"]) | ||||||
| self.assertIn("No endpoint URL configured", data["status"]["error"]) | ||||||
| self.assertIsNone(data["status"]["endpoint_url"]) | ||||||
|
|
||||||
| def test_get_status_with_null_endpoint_url(self): | ||||||
| """Test get_status method when endpoint_url is None""" | ||||||
| service = ProcessingService.objects.create(name="Pull Mode Service", endpoint_url=None) | ||||||
| service.projects.add(self.project) | ||||||
|
|
||||||
| status = service.get_status() | ||||||
|
|
||||||
| self.assertFalse(status.request_successful) | ||||||
| self.assertIsNone(status.server_live) | ||||||
| self.assertIsNone(status.endpoint_url) | ||||||
| self.assertIsNotNone(status.error) | ||||||
| self.assertTrue("No endpoint URL configured" in (status.error or "")) | ||||||
|
||||||
| self.assertTrue("No endpoint URL configured" in (status.error or "")) | |
| self.assertIn("No endpoint URL configured", (status.error or "")) |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -52,6 +52,7 @@ django-anymail[sendgrid]==10.0 # https://github.com/anymail/django-anymail | |||
| Werkzeug[watchdog]==2.3.6 # https://github.com/pallets/werkzeug | ||||
| ipdb==0.13.13 # https://github.com/gotcha/ipdb | ||||
| psycopg[binary]==3.1.9 # https://github.com/psycopg/psycopg | ||||
| # psycopg==3.1.9 # https://github.com/psycopg/psycopg # the non-binary version is needed for some platforms | ||||
|
||||
| # psycopg==3.1.9 # https://github.com/psycopg/psycopg # the non-binary version is needed for some platforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When endpoint_url is None, the method returns early without updating the last_checked, last_checked_live, and last_checked_latency fields. This means services without endpoint URLs will never have these tracking fields updated. Consider updating these fields even for null endpoint services to maintain consistency and allow tracking when the service was last checked, even if it returned a "no endpoint" status. For example, set last_checked to the current time and last_checked_live to False.