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

Development: Fix default URL for telemetry service #9382

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

b-fein
Copy link
Contributor

@b-fein b-fein commented Sep 29, 2024

Checklist

General

Motivation and Context

With the current default URL, the telemetry sending did not work:

artemis | 2024-09-28T16:08:58.749Z ERROR 1 --- [Artemis] [ artemis-task-1] .a.i.SimpleAsyncUncaughtExceptionHandler : Unexpected exception occurred invoking async method: public void de.tum.cit.aet.artemis.core.service.TelemetrySendingService.sendTelemetryByPostRequest() throws java.lang.Exception
artemis | 
artemis | java.lang.IllegalArgumentException: URI is not absolute
artemis | 	at java.base/java.net.URL.of(URL.java:862)
artemis | 	at java.base/java.net.URI.toURL(URI.java:1172)
artemis | 	at org.springframework.http.client.SimpleClientHttpRequestFactory.createRequest(SimpleClientHttpRequestFactory.java:154)

Description

Adds the https:// at the start to turn it into a proper URL.

Related PR: ls1intum/artemis-ansible-collection#102

Steps for Testing

Start a server in production config. It should not log the error above.

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Performance Review

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance even for very large courses with more than 2000 students.
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

unchanged

@coderabbitai ignore

@github-actions github-actions bot added documentation config-change Pull requests that change the config in a way that they require a deployment via Ansible. labels Sep 29, 2024
@b-fein b-fein marked this pull request as ready for review September 29, 2024 07:17
@b-fein b-fein requested a review from a team as a code owner September 29, 2024 07:17
@b-fein b-fein added this to the 7.5.5 milestone Sep 29, 2024
Copy link
Contributor

@SimonEntholzer SimonEntholzer left a comment

Choose a reason for hiding this comment

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

Tested locally, that fixes the issue.

@krusche krusche merged commit 8f73d05 into develop Sep 29, 2024
35 of 39 checks passed
@krusche krusche deleted the bugfix/development/fix-telemetry-default-url branch September 29, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix config-change Pull requests that change the config in a way that they require a deployment via Ansible. documentation ready for review small
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

3 participants