-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
Add API for triggering a speedtest. #1141
base: implement-an-api
Are you sure you want to change the base?
Add API for triggering a speedtest. #1141
Conversation
Im also thinking about securing the API with an Key, but i saw that the old enpoint was unsecured, so i didnt secure the new as well. What do you think about? |
Couple of thoughts so bare with me as I respond while trying to feed my daughter. I also noticed there are a couple "new" features in this PR, my preference is to have these in separate PR's as they make reviewing code and testing features easier. My first couple of comments come from that thought. Feature feedback
API feedback
|
To your point about "many new features", I agree that the PR contains more than the API, but we need some way for the API caller to track the triggered speedtest, because we cannot just wait for the results to be returned, because with slow internet connections we expect timeouts. The caller then has no way to get information about the triggered test. Personally, I would use a separate table for tracking speedtests, because I think it better meets the 2NF, and of course the result table should only contain information about the result of the speedtest. API Feedback
Can you please summarize what should be left of this feature? |
Getting the quick one out of the way, the existing API endpoint should be left as-is. Homepage expects a response at that URI with the given payload already. For tracking the status of a requested speedtest walk me through the benefits of using 2NF and keeping history over not. I'm trying to keep an eye to simplicity (K.I.S.S.) so if we're going to add data and complexity I want to make sure it's going to be useful. Edit: for anyone else who reads this and wonders what "2NF" is this stands for normal forms regarding the data structure inside of the database. |
I did not touch the existing API endpoint. It still returns the same resource in the same path. I just changed the location of the code, from a separate controller to one that also handles other requests. A separate table allows you to quickly search the status of a speedtest. Keeping the job tracking information separate from the results data helps maintain a clear separation of concerns. This separation can make the schema easier to understand and maintain, especially as the system grows in complexity. A separate table allows you to keep a history of all job attempts, statuses, and changes over time, which can be invaluable for debugging, auditing, and analysis. A separate job_tracking table is easier to extend in the future with additional tracking-related attributes (e.g. started_at, finished_at, error_message) without cluttering the results table. |
Just doing a little house keeping, can you update your branch from I also need to get you a priorities list which is why I changed the base branch so we have a "working" branch. |
Did the merge, but now we have the status duplicated. |
I haven't, I've been focused hardcore on bringing stability back to v0.16.x and squashing bugs. Give me a couple of days to get through client work so I have the mental capacity to give this the time it needs. |
@ChristophKronberger #1264 has some pretty fundamental changes into how the process works for running manual and scheduled speedtests. My goal is to make the process standardized and more flexible when triggering manual or scheduled speedtests. This also includes when I go to implement multiple speedtest services (i.e. iPerf). My hope is this makes hooking into the process a whole lot easier. Once this is wrapped up we should be able to put a nail in how we're going to track the status of a speedtest run and hook the API into this process to easily trigger a speedtest. |
@ChristophKronberger can you resolve conflicts? I want to get this merged into the api branch so I can leverage the work you've done and expand on it for the upcoming API. |
@alexjustesen |
📃 Description
Added support for triggering speedtests using an API post request.
The client sends a POST request to /api/speedtest, and the API responds with a uuid that maps to a speedtest task.
The client can get the status of the task with a get request to /api/speedtest/trackingid/{id}.
The server responds with the current status of the speedtest, if not finished, or with the result of the speedtest.
Closes: #1047
🪵 Changelog
➕ Added
✏️ Changed
Removed 🗑️