Skip to content

Commit

Permalink
Merge pull request #12096 from thesujai/networkclient-instead-of-requ…
Browse files Browse the repository at this point in the history
…ests

Replace requests with NetworkClient
  • Loading branch information
rtibbles authored Aug 7, 2024
2 parents acacabf + 66cb5d0 commit 79e9e4b
Show file tree
Hide file tree
Showing 23 changed files with 242 additions and 206 deletions.
37 changes: 18 additions & 19 deletions kolibri/core/analytics/measurements.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from collections import namedtuple
from datetime import timedelta

import requests
from django.contrib.sessions.models import Session
from django.db import connection
from django.db.models import Count
Expand All @@ -12,6 +11,8 @@

from kolibri.core.analytics import SUPPORTED_OS
from kolibri.core.content.models import ChannelMetadata
from kolibri.core.discovery.utils.network.client import NetworkClient
from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure
from kolibri.core.logger.models import ContentSessionLog
from kolibri.core.logger.models import UserSessionLog
from kolibri.utils.server import NotRunning
Expand Down Expand Up @@ -112,27 +113,25 @@ def get_requests_info():
- Kolibri channels list
"""

def format_url(url, base_url):
formatted = "{base_url}{url}".format(base_url=base_url, url=url)
return formatted

_, port = get_kolibri_process_info()
if port:

def get_time(url):
base_url = "http://localhost:{}".format(port)
homepage_time = "{:.2f} s".format(
requests.get(base_url).elapsed.total_seconds()
)
recommended_url = format_url(
"/api/content/contentnode_slim/popular/?include_coach_content=false",
base_url,
)
recommended_time = "{:.2f} s".format(
requests.get(recommended_url).elapsed.total_seconds()
)
channels_url = format_url("/api/content/channel/?available=true", base_url)
channels_time = "{:.2f} s".format(
requests.get(channels_url).elapsed.total_seconds()
client = NetworkClient.build_for_address(base_url)
try:
response = client.get(url)
return response.elapsed.total_seconds()
except NetworkLocationResponseFailure as e: # most probably a 404
return e.response.elapsed.total_seconds()

if port:
homepage_time = "{:.2f} s".format(get_time("/"))
recommended_url = (
"/api/content/contentnode_slim/popular/?include_coach_content=false"
)
recommended_time = "{:.2f} s".format(get_time(recommended_url))
channels_url = "/api/content/channel/?available=true"
channels_time = "{:.2f} s".format(get_time(channels_url))
else:
homepage_time = recommended_time = channels_time = None

Expand Down
12 changes: 6 additions & 6 deletions kolibri/core/analytics/tasks.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import logging

from django.db import connection
from requests.exceptions import ConnectionError
from requests.exceptions import RequestException
from requests.exceptions import Timeout

from kolibri.core.analytics.utils import DEFAULT_SERVER_URL
from kolibri.core.analytics.utils import ping_once
from kolibri.core.discovery.utils.network.errors import NetworkLocationConnectionFailure
from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure
from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseTimeout
from kolibri.core.tasks.decorators import register_task
from kolibri.core.tasks.exceptions import JobRunning
from kolibri.core.tasks.main import job_storage
Expand All @@ -25,21 +25,21 @@
def _ping(started, server, checkrate):
try:
ping_once(started, server=server)
except ConnectionError:
except NetworkLocationConnectionFailure:
logger.warning(
"Ping failed (could not connect). Trying again in {} minutes.".format(
checkrate
)
)
raise
except Timeout:
except NetworkLocationResponseTimeout:
logger.warning(
"Ping failed (connection timed out). Trying again in {} minutes.".format(
checkrate
)
)
raise
except RequestException as e:
except NetworkLocationResponseFailure as e:
logger.warning(
"Ping failed ({})! Trying again in {} minutes.".format(e, checkrate)
)
Expand Down
16 changes: 9 additions & 7 deletions kolibri/core/analytics/test/test_ping.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,30 @@ def load_zipped_json(data):
return json.loads(data)


def mocked_requests_post_wrapper(json_data, status_code):
def mocked_requests_post(*args, **kwargs):
def mocked_network_client_post_wrapper(json_data, status_code):
def mocked_network_client_post(*args, **kwargs):
class MockResponse(Response):
def __init__(self):
self.json_data = json_data
self.status_code = status_code
self._content = json.dumps(json_data).encode()
self.reason = ""
self.url = args[0]
if 400 <= self.status_code < 600:
self.raise_for_status()

def json(self):
return self.json_data

return MockResponse()

return mocked_requests_post
return mocked_network_client_post


class PingCommandTestCase(BaseDeviceSetupMixin, TransactionTestCase):
@mock.patch(
"kolibri.core.analytics.utils.requests.post",
side_effect=mocked_requests_post_wrapper({"id": 17}, 200),
"kolibri.core.discovery.utils.network.client.NetworkClient.post",
side_effect=mocked_network_client_post_wrapper({"id": 17}, 200),
)
def test_ping_succeeds(self, post_mock):
call_command("ping", once=True)
Expand All @@ -49,8 +51,8 @@ def test_ping_succeeds(self, post_mock):
assert load_zipped_json(post_mock.call_args_list[1][1]["data"])["pi"] == 17

@mock.patch(
"kolibri.core.analytics.utils.requests.post",
side_effect=mocked_requests_post_wrapper({}, 400),
"kolibri.core.discovery.utils.network.client.NetworkClient.post",
side_effect=mocked_network_client_post_wrapper({}, 400),
)
def test_ping_fails(self, post_mock):
with self.assertRaises(CommandError):
Expand Down
16 changes: 8 additions & 8 deletions kolibri/core/analytics/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import logging
import math

import requests
from dateutil import parser
from django.core.serializers.json import DjangoJSONEncoder
from django.db import transaction
Expand All @@ -32,6 +31,7 @@
from kolibri.core.content.models import LocalFile
from kolibri.core.device.utils import allow_guest_access
from kolibri.core.device.utils import get_device_setting
from kolibri.core.discovery.utils.network.client import NetworkClient
from kolibri.core.exams.models import Exam
from kolibri.core.lessons.models import Lesson
from kolibri.core.logger.models import AttemptLog
Expand All @@ -40,7 +40,6 @@
from kolibri.core.logger.models import MasteryLog
from kolibri.core.logger.models import UserSessionLog
from kolibri.core.utils.lock import db_lock
from kolibri.core.utils.urls import join_url
from kolibri.utils import conf
from kolibri.utils.server import installation_type
from kolibri.utils.time_utils import local_now
Expand Down Expand Up @@ -415,7 +414,9 @@ def create_and_update_notifications(data, source):

def perform_ping(started, server=DEFAULT_SERVER_URL):

url = join_url(server, "/api/v1/pingback")
client = NetworkClient(server)

url = "/api/v1/pingback"

instance, _ = InstanceIDModel.get_or_create_current_instance()

Expand Down Expand Up @@ -447,20 +448,19 @@ def perform_ping(started, server=DEFAULT_SERVER_URL):

logger.debug("Pingback data: {}".format(data))
jsondata = dump_zipped_json(data)
response = requests.post(url, data=jsondata, timeout=60)
response.raise_for_status()
response = client.post(url, data=jsondata, timeout=60)
return json.loads(response.content.decode() or "{}")


def perform_statistics(server, pingback_id):
url = join_url(server, "/api/v1/statistics")
client = NetworkClient(server)
url = "/api/v1/statistics"
channels = [extract_channel_statistics(c) for c in ChannelMetadata.objects.all()]
facilities = [extract_facility_statistics(f) for f in Facility.objects.all()]
data = {"pi": pingback_id, "c": channels, "f": facilities}
logger.debug("Statistics data: {}".format(data))
jsondata = dump_zipped_json(data)
response = requests.post(url, data=jsondata, timeout=60)
response.raise_for_status()
response = client.post(url, data=jsondata, timeout=60)
return json.loads(response.content.decode() or "{}")


Expand Down
19 changes: 11 additions & 8 deletions kolibri/core/api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import uuid

import requests
from django.http import Http404
from django.http.request import QueryDict
from rest_framework import viewsets
Expand All @@ -20,7 +19,9 @@

from .utils.portal import registerfacility
from kolibri.core.auth.models import Facility
from kolibri.core.utils.urls import join_url
from kolibri.core.discovery.utils.network.client import NetworkClient
from kolibri.core.discovery.utils.network.errors import NetworkLocationConnectionFailure
from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure
from kolibri.utils import conf


Expand All @@ -30,7 +31,7 @@ def register(self, request):
facility = Facility.objects.get(id=request.data.get("facility_id"))
try:
response = registerfacility(request.data.get("token"), facility)
except requests.exceptions.RequestException as e: # bubble up any response error
except NetworkLocationResponseFailure as e: # bubble up any response error
return Response(e.response.json(), status=e.response.status_code)
return Response(status=response.status_code)

Expand All @@ -39,14 +40,16 @@ def validate_token(self, request):
PORTAL_URL = conf.OPTIONS["Urls"]["DATA_PORTAL_SYNCING_BASE_URL"]
try:
# token is in query params
response = requests.get(
join_url(
PORTAL_URL, "portal/api/public/v1/registerfacility/validate_token"
),
client = NetworkClient(PORTAL_URL)
response = client.get(
"portal/api/public/v1/registerfacility/validate_token",
params=request.query_params,
)
except requests.exceptions.ConnectionError:
except NetworkLocationConnectionFailure:
return Response({"status": "offline"}, status=HTTP_503_SERVICE_UNAVAILABLE)
except NetworkLocationResponseFailure as e:
# bubble up for any other response error
response = e.response
# handle any invalid json type responses
try:
data = response.json()
Expand Down
10 changes: 5 additions & 5 deletions kolibri/core/auth/management/commands/fullfacilitysync.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import getpass

import requests
from django.core.exceptions import ValidationError
from django.core.management import call_command
from django.core.management.base import CommandError
Expand All @@ -18,8 +17,9 @@
from kolibri.core.device.models import DevicePermissions
from kolibri.core.device.utils import device_provisioned
from kolibri.core.device.utils import provision_device
from kolibri.core.discovery.utils.network.client import NetworkClient
from kolibri.core.tasks.management.commands.base import AsyncCommand
from kolibri.core.utils.urls import reverse_remote
from kolibri.core.utils.urls import reverse_path


class Command(AsyncCommand):
Expand All @@ -34,9 +34,9 @@ def add_arguments(self, parser):

def get_dataset_id(self, base_url, dataset_id):
# get list of facilities and if more than 1, display all choices to user
facility_url = reverse_remote(base_url, "kolibri:core:publicfacility-list")
facility_resp = requests.get(facility_url)
facility_resp.raise_for_status()
client = NetworkClient.build_for_address(base_url)
facility_url = reverse_path("kolibri:core:publicfacility-list")
facility_resp = client.get(facility_url)
facilities = facility_resp.json()
if len(facilities) > 1 and not dataset_id:
message = "Please choose a facility to sync with:\n"
Expand Down
7 changes: 4 additions & 3 deletions kolibri/core/auth/management/commands/registerfacility.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
from django.core.management.base import BaseCommand
from django.core.management.base import CommandError
from morango.models import Certificate
from requests import exceptions

from kolibri.core import error_constants
from kolibri.core.auth.management.utils import confirm_or_exit
from kolibri.core.auth.management.utils import get_facility
from kolibri.core.discovery.utils.network.errors import NetworkClientError
from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure
from kolibri.core.utils.portal import registerfacility


Expand Down Expand Up @@ -42,7 +43,7 @@ def handle(self, *args, **options):
)
)
# an invalid nonce/register response
except exceptions.HTTPError as e:
except NetworkLocationResponseFailure as e:
error = e.response.json()[0]
message = error["metadata"].get("message") or e.response.text
# handle facility not existing response from portal server
Expand Down Expand Up @@ -71,5 +72,5 @@ def handle(self, *args, **options):
)
)
# handle any other invalid response
except exceptions.RequestException as e:
except NetworkClientError as e:
raise CommandError(e)
9 changes: 4 additions & 5 deletions kolibri/core/auth/management/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from contextlib import contextmanager
from functools import wraps

import requests
from django.core.management.base import CommandError
from morango.models import Certificate
from morango.models import InstanceIDModel
Expand Down Expand Up @@ -37,7 +36,7 @@
from kolibri.core.tasks.exceptions import UserCancelledError
from kolibri.core.tasks.management.commands.base import AsyncCommand
from kolibri.core.utils.lock import db_lock_sqlite_only
from kolibri.core.utils.urls import reverse_remote
from kolibri.core.utils.urls import reverse_path
from kolibri.utils.data import bytes_for_humans


Expand Down Expand Up @@ -125,9 +124,9 @@ def get_facility(facility_id=None, noninteractive=False):

def get_facility_dataset_id(baseurl, identifier=None, noninteractive=False):
# get list of facilities and if more than 1, display all choices to user
facility_url = reverse_remote(baseurl, "kolibri:core:publicfacility-list")
response = requests.get(facility_url)
response.raise_for_status()
client = NetworkClient.build_for_address(baseurl)
facility_url = reverse_path("kolibri:core:publicfacility-list")
response = client.get(facility_url)
facilities = response.json()
if not facilities:
raise CommandError("There are no facilities available at: {}".format(baseurl))
Expand Down
9 changes: 5 additions & 4 deletions kolibri/core/auth/test/sync_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
import time
import uuid

import requests
from django.conf import settings
from django.db import connection
from django.db import connections
from django.utils.functional import wraps
from morango.models.core import DatabaseIDModel
from requests.exceptions import RequestException

from kolibri.core.auth.models import Facility
from kolibri.core.auth.models import FacilityUser
from kolibri.core.discovery.utils.network.client import NetworkClient
from kolibri.core.discovery.utils.network.errors import NetworkLocationResponseFailure

# custom Morango instance info used in tests
CUSTOM_INSTANCE_INFO = {"kolibri": "0.14.7"}
Expand Down Expand Up @@ -119,12 +119,13 @@ def pipe_shell(self, text):
)

def _wait_for_server_start(self, timeout=20):
client = NetworkClient.build_for_address(self.baseurl, timeout=3)
for i in range(timeout * 2):
try:
resp = requests.get(self.baseurl + "api/public/info/", timeout=3)
resp = client.get("api/public/info/")
if resp.status_code > 0:
return
except RequestException:
except NetworkLocationResponseFailure:
pass
time.sleep(0.5)

Expand Down
2 changes: 1 addition & 1 deletion kolibri/core/auth/test/test_register_facility.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def setUp(self):
self.token = "token"
FacilityFactory()

@mock.patch("requests.post")
@mock.patch("kolibri.core.discovery.utils.network.client.NetworkClient.post")
def test_no_owned_certificates(self, post_mock):
post_mock.json.return_value = {"id": uuid.uuid4().hex}
cert = Certificate.objects.first()
Expand Down
Loading

0 comments on commit 79e9e4b

Please sign in to comment.