Reserved network locations for Studio and KDP #12703

LianaHarris360 · 2024-10-03T21:14:53Z

Summary

This pull request allows the use of NetworkLocation models and associated architecture to interact with Studio and KDP.

Details

Removes dynamic field and its setter from model NetworkLocation after migrating to field location_type.
Updates the task reset_connection_states to create or update reserved network locations for Studio and KDP using URLs from Kolibri's options.ini file. Uses hardcoded IDs for these locations, resets their connection states similarly to static locations, and for KDP locations, sets the application to Kolibri Data Portal without enqueuing a perform_network_location_update task.
Updates the viewset and serializer for NetworkLocations model to annotate a dynamic attribute on the response when location_type is dynamic, only allow static models to be created (and set the location_type to static when creating them), and exclude KDP and Studio if syncable filter is not provided.

Notes

As noted in this comment, KDP does respond to information requests; so preventing request to KDP's location within the method update_connection_status was not implemented.
Studio lacks an instance ID, so we hash the URL to generate one. This approach doesn't work for KDP, which already has an instance ID not derived from the URL. The ID can be hardcoded, but it can't be a UUID generated from the URL like in Studio.

References

Closes #10431

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

…_type

…o and KDP

…eated and set location_type to static

…rkLocationViewSet

…io reserved location, excluding both if syncable is not provided

…cal is false and update previous tests that used dynamic field

github-actions · 2024-10-03T21:47:19Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.18.0.dev0_git.20241120133716.pex
Windows Installer (EXE)	kolibri-0.18.0.dev0+git.20241120133716-windows-setup-unsigned.exe
Debian Package	kolibri_0.18.0.dev0+git.20241120133716-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.18.0.dev0+git.20241120133716.dmg
Android Package (APK)	kolibri-0.18.0.dev0+git.20241120133716-0.1.4-debug.apk
TAR file	kolibri-0.18.0.dev0+git.20241120133716.tar.gz
WHL file	kolibri-0.18.0.dev0+git.20241120133716-py2.py3-none-any.whl

pcenov · 2024-10-07T12:15:00Z

Hi @LianaHarris360 and @marcellamaki no issues observed while regression testing the content import from studio and syncing to KDP.

bjester

Mainly just questions, as I'm not sure I understand all of the reasoning behind some of the changes.

kolibri/core/discovery/tasks.py

kolibri/core/discovery/api.py

kolibri/core/discovery/serializers.py

kolibri/core/discovery/well_known.py

kolibri/core/discovery/api.py

kolibri/core/discovery/tasks.py

akolson

Changes look correct to me! It appears there is just one clarification to take care of, otherwise all everything else seems fine to me. I'll leave the final approval to @bjester. Thanks @LianaHarris360

bjester

Thank you for addressing the feedback so far. There's just one lingering issue with the API

kolibri/core/discovery/serializers.py

bjester · 2024-11-18T20:13:08Z

kolibri/core/discovery/api.py

+        if syncable == "1":
+            # Include KDP's reserved location
+            queryset = NetworkLocation.objects.filter(
+                Q(location_type=LocationTypes.Static)
+                | Q(id=DATA_PORTAL_BASE_INSTANCE_ID)
+            )
+        elif syncable == "0":
+            # Include Studio's reserved location
+            queryset = NetworkLocation.objects.filter(
+                Q(location_type=LocationTypes.Static)
+                | Q(id=CENTRAL_CONTENT_BASE_INSTANCE_ID)
+            )


This isn't quite 100% to what we need. For background, the reasoning behind this filter is to return locations that either are syncable (for facility data syncing) or can be used for content import. Studio can be used for content import but syncing. KDP can be used for syncing but not content import. As coded, this wouldn't return dynamic locations, which are similar to static in that we assume they can do both.

So this filter should consistently return static and dynamic locations, and only include KDP/Studio reserved locations based on the syncable value?

… locations

bjester

LGTM

LianaHarris360 added 9 commits October 3, 2024 14:47

Remove dynamic field and its setter after migrating to field location…

41b0989

…_type

Update task helper to prevent enqueuing new tasks when is_local is False

8b4b872

Use hardcoded ID to create reserved network locations for KDP

9704b4a

Update reset_connection_states to handle reserved locations for Studi…

8a96989

…o and KDP

Update NetworkLocationSerializer to only allow static models to be cr…

b9eb839

…eated and set location_type to static

Annotate dynamic attribute when location_type is dynamic within Netwo…

9976dd6

…rkLocationViewSet

Add syncable filter in NetworkLocationViewSet to include KDP and Stud…

5ffd66c

…io reserved location, excluding both if syncable is not provided

Change DATA_PORTAL_INSTANCE_ID to DATA_PORTAL_BASE_INSTANCE_ID

5698dc9

Add test for enqueue_network_location_update_with_backoff when not_lo…

4b785f0

…cal is false and update previous tests that used dynamic field

LianaHarris360 added the DEV: backend Python, databases, networking, filesystem... label Oct 3, 2024

LianaHarris360 added this to the Kolibri 0.18: General maintenance milestone Oct 3, 2024

LianaHarris360 requested review from bjester, rtibbles and akolson October 3, 2024 21:14

Remove use of dynamic field for NetworkLocation model

c1a5767

bjester self-assigned this Oct 8, 2024

bjester reviewed Oct 11, 2024

View reviewed changes

LianaHarris360 added 4 commits October 22, 2024 14:02

Remove filtering for KDP network locations before enqueuing

d51a97d

Enforce NetworkLocationSerializer to use StaticNetworkLocation model

c4830a9

Add correct KDP instance_id

7d0de1b

Return dynamic locations in addition to static ones

5dcb456

LianaHarris360 requested a review from bjester October 24, 2024 14:13

akolson reviewed Nov 11, 2024

View reviewed changes

kolibri/core/discovery/tasks.py Outdated Show resolved Hide resolved

akolson reviewed Nov 11, 2024

View reviewed changes

LianaHarris360 added 2 commits November 13, 2024 10:21

Remove duplicate KDP application setting

4969474

Remove annotated dynamic attribute

2d2b5af

bjester requested changes Nov 18, 2024

View reviewed changes

Refactor NetworkLocationViewSet to return dynamic locations

9aa66e1

Remove NetworkLocationViewSet get_queryset redundancy

a425e4f

LianaHarris360 requested a review from bjester November 19, 2024 16:10

Add API test cases for returned static, dynamic, and reserved network…

b97e1d3

… locations

bjester approved these changes Nov 20, 2024

View reviewed changes

bjester merged commit d29abf8 into learningequality:develop Nov 20, 2024
35 checks passed

bjester mentioned this pull request Nov 22, 2024

Update content request handling to not use just any reserved location #12876

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reserved network locations for Studio and KDP #12703

Reserved network locations for Studio and KDP #12703

LianaHarris360 commented Oct 3, 2024

github-actions bot commented Oct 3, 2024 •

edited

Loading

pcenov commented Oct 7, 2024

bjester left a comment

akolson left a comment

bjester left a comment

bjester Nov 18, 2024

LianaHarris360 Nov 19, 2024

bjester Nov 19, 2024

bjester left a comment

Reserved network locations for Studio and KDP #12703

Reserved network locations for Studio and KDP #12703

Conversation

LianaHarris360 commented Oct 3, 2024

Summary

Details

Notes

References

Testing checklist

PR process

Reviewer checklist

github-actions bot commented Oct 3, 2024 • edited Loading

Build Artifacts

pcenov commented Oct 7, 2024

bjester left a comment

Choose a reason for hiding this comment

akolson left a comment

Choose a reason for hiding this comment

bjester left a comment

Choose a reason for hiding this comment

bjester Nov 18, 2024

Choose a reason for hiding this comment

LianaHarris360 Nov 19, 2024

Choose a reason for hiding this comment

bjester Nov 19, 2024

Choose a reason for hiding this comment

bjester left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 3, 2024 •

edited

Loading