Skip to content

Commit

Permalink
[BUGFIX] Fix exporter test button #263
Browse files Browse the repository at this point in the history
- Query based on project.id (instead of farm.id) for future refactoring
- Change from form to regular POST data to avoid extra validation confusion
- Simplify exporter-test button html
  • Loading branch information
kfdm authored Mar 31, 2020
2 parents cb5aff5 + 23f6658 commit 17034c5
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 48 deletions.
26 changes: 6 additions & 20 deletions promgen/templates/promgen/exporter_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,9 @@ <h1>Project: {{ project.name }}</h1>
<div class="panel-footer">
<div class="input-group-btn">
<button class="btn btn-primary">Register Exporter</button>
<exporter-test
class="btn btn-info"
{% if project.farm %}
href="{% url 'exporter-scrape' project.farm.id %}"
{% else %}
title="Please assign farm to test"
disabled
{% endif %}
form="#exporter_form"
target="#exporterresult">{% trans "Test" %}</exporter-test>
<exporter-test class="btn btn-info" href="{% url 'exporter-scrape' project.id %}" target="#exporterresult">
{% trans "Test" %}
</exporter-test>
</div>
</div>
</div>
Expand All @@ -67,16 +60,9 @@ <h1>Project: {{ project.name }}</h1>
<input type="hidden" name="enabled" value="1" />
<div class="input-group-btn">
<button style="width:80%" class="btn btn-primary">Register {{ default.job }} :{{ default.port }}{{ default.path }}</button>
<exporter-test
style="width:20%"
class="btn btn-info"
{% if project.farm %}
href="{% url 'exporter-scrape' project.farm.id %}"
{% else %}
title="Please assign farm to test"
disabled
{% endif %}
target="#exporterresult">{% trans "Test" %}</exporter-test>
<exporter-test class="btn btn-info" href="{% url 'exporter-scrape' project.id %}" target="#exporterresult">
{% trans "Test" %}
</exporter-test>
</div>
</form>
{% endfor %}
Expand Down
14 changes: 3 additions & 11 deletions promgen/templates/promgen/project_detail_exporters.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,9 @@
<input type="hidden" name="scheme" value="{{ exporter.scheme }}" />
<input type="hidden" name="port" value="{{ exporter.port }}" />
<input type="hidden" name="path" value="{{ exporter.path }}" />
<exporter-test
class="btn btn-info btn-xs"
{% if project.farm %}
href="{% url 'exporter-scrape' project.farm.id %}"
{% else %}
title="Please assign farm to test"
disabled
{% endif %}
target="#exporterresult"
>{% trans "Test" %}</exporter-test>
</div>
<exporter-test class="btn btn-info btn-xs" href="{% url 'exporter-scrape' project.id %}" target="#exporterresult">
{% trans "Test" %}
</exporter-test>
</form>
</td>
<td>
Expand Down
9 changes: 7 additions & 2 deletions promgen/tests/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,13 @@ def test_hosts(self):

@mock.patch("promgen.util.get")
def test_scrape(self, mock_get):
farm = models.Farm.objects.create(name="test_scrape")
shard = models.Shard.objects.create(name="test_scrape_shard")
service = models.Service.objects.create(name="test_scrape_service")
farm = models.Farm.objects.create(name="test_scrape_farm")
farm.host_set.create(name="example.com")
project = models.Project.objects.create(
name="test_scrape_project", service=service, shard=shard, farm=farm
)

# Uses the scrape target as the key, and the POST body that should
# result in that URL
Expand Down Expand Up @@ -109,7 +114,7 @@ def test_scrape(self, mock_get):
# For each POST body, check to see that we generate and attempt to
# scrape the correct URL
response = self.client.post(
reverse("exporter-scrape", args=(farm.pk,)), body
reverse("exporter-scrape", kwargs={"pk": project.pk}), body
)
self.assertRoute(response, views.ExporterScrape, 200)
self.assertEqual(mock_get.call_args[0][0], url)
Expand Down
2 changes: 1 addition & 1 deletion promgen/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
path('project/<int:pk>/newfarm', views.FarmRegister.as_view(), name='farm-new'),
path('project/<int:pk>/exporter', views.ExporterRegister.as_view(), name='project-exporter'),
path('project/<int:pk>/notifier', views.ProjectNotifierRegister.as_view(), name='project-notifier'),
path('project/<int:pk>/scrape', views.ExporterScrape.as_view(), name='exporter-scrape'),

path('exporter/<int:pk>/delete', views.ExporterDelete.as_view(), name='exporter-delete'),
path('exporter/<int:pk>/toggle', views.ExporterToggle.as_view(), name='exporter-toggle'),
Expand All @@ -63,7 +64,6 @@

path('farm', views.FarmList.as_view(), name='farm-list'),
path('farm/<int:pk>', views.FarmDetail.as_view(), name='farm-detail'),
path('farm/<int:pk>/scrape', views.ExporterScrape.as_view(), name='exporter-scrape'),
path('farm/<int:pk>/refresh', views.FarmRefresh.as_view(), name='farm-refresh'),
path('farm/<int:pk>/hosts', views.HostRegister.as_view(), name='hosts-add'),
path('farm/<int:pk>/update', views.FarmUpdate.as_view(), name='farm-update'),
Expand Down
29 changes: 15 additions & 14 deletions promgen/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,32 +615,30 @@ def form_valid(self, form):
exporter, _ = models.Exporter.objects.get_or_create(project=project, **form.clean())
return HttpResponseRedirect(reverse('project-detail', args=[project.id]))

class ExporterScrape(LoginRequiredMixin, FormView):

class ExporterScrape(LoginRequiredMixin, View):
# TODO: Move to /rest/project/<slug>/scrape
form_class = forms.ExporterForm
def post(self, request, pk):
# Lookup our farm for testing
farm = get_object_or_404(models.Project, pk=pk).farm

def form_invalid(self, form):
return JsonResponse(
{"message": "Form Errors", "errors": form.errors}, status=400
)

def form_valid(self, form):
futures = []
farm = get_object_or_404(models.Farm, id=self.kwargs["pk"])
# So we have a mutable dictionary
data = request.POST.dict()

# The default __metrics_path__ for Prometheus is /metrics so we need to
# manually add it here in the case it's not set for our test
if not form.cleaned_data["path"]:
form.cleaned_data["path"] = "/metrics"
if not data.setdefault("path", "/metrics"):
data["path"] = "/metrics"

def query():
futures = []
with concurrent.futures.ThreadPoolExecutor(max_workers=20) as executor:
for host in farm.host_set.all():
futures.append(
executor.submit(
util.get,
"{scheme}://{host}:{port}{path}".format(
host=host.name, **form.cleaned_data
host=host.name, **data
),
)
)
Expand All @@ -659,7 +657,10 @@ def query():
logger.exception("Unknown Exception")
yield "Unknown URL", "Unknown error"

return JsonResponse(dict(query()))
try:
return JsonResponse(dict(query()))
except Exception as e:
return JsonResponse({"error": "Error with query %s" % e})


class URLRegister(LoginRequiredMixin, FormView, mixins.ProjectMixin):
Expand Down

0 comments on commit 17034c5

Please sign in to comment.