[DRAFT] Adds serverlist api #93
Conversation
| ## TODO: ADD VALIDATION FOR APPROVED API KEYS SO THAT STRANGERS DONT SPAM STATIONHUB WITH INVALID SERVERS ## | ||
| for server in servers: | ||
| if server.is_expired(): | ||
| server.delete() |
There was a problem hiding this comment.
This seems expensive to delete each one separately
Can it be done in 1 operation?
I also suspect race condition is present. Imagine 2 requests coming in at the same time. Both connections try deleting same server. will it error?
| server_validation = validate_server_info(request) | ||
| if server_validation.status_code != 200: | ||
| return server_validation | ||
| data = json.loads(request.body) |
There was a problem hiding this comment.
django doesn't have ability to create model from json? weird code
| if existing_server: | ||
| # In case the server updates its info quickly before the 10 seconds pass, | ||
| # we update everything while we're updating the experitation date as well | ||
| existing_server.ip_address = ip_address |
There was a problem hiding this comment.
this is wrong because .get() can return None
| existing_server.updated_at = datetime.now() | ||
| existing_server.save() | ||
| else: | ||
| Server.objects.create( |
There was a problem hiding this comment.
This will error because .get() can return None
| image_link=image_link, | ||
| updated_at=datetime.now(), | ||
| ) | ||
| return JsonResponse({"success": "Server data added/updated successfully"}, status=200) |
There was a problem hiding this comment.
you don't need this json body
| updated_at=datetime.now(), | ||
| ) | ||
| return JsonResponse({"success": "Server data added/updated successfully"}, status=200) | ||
| except Exception as e: |
There was a problem hiding this comment.
enormous kilometer wide try block
| ): | ||
| return JsonResponse({"error": "Missing build information."}, status=400) | ||
|
|
||
| return JsonResponse({"success": "Server info validated successfully"}, status=200) |
There was a problem hiding this comment.
you don't need this json body
|
|
||
| class Server(models.Model): | ||
| name = models.CharField(max_length=100) | ||
| ip_address = models.CharField(max_length=50) |
There was a problem hiding this comment.
Maximum length of a domain part is 63 characters, and an entire FQDN is 253. 50 could un-necessarily limit domains being used.
For example, cpee0dbd14eedcc-cme0dbd14eedca.cpe.net.cable.rogers.com is a real hostname (I found on Google) that measures >50 chars.
|
I started working this on #95 |
Still WIP, intended to replace the current serverlist api and move it over to centeral command.
unitystation.orgunitystation.org