Content import peer discovery #6011

indirectlylit · 2019-10-30T23:24:36Z

Summary

builds off of #5660

Reviewer guidance

Work-in-progress

References

#5660

Contributor Checklist

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

Testing:

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

Reviewer Checklist

Automated test coverage is satisfactory
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

(also means that we start with a soft interrupt and only hard-kill later if needed, as we do on Linux)

micahscopes · 2019-11-06T23:01:38Z

kolibri/core/discovery/utils/network/search.py

+    """Retrieve a list of dicts with information about the discovered Kolibri instances on the local network,
+    filtering out those that can't be accessed at the specified port (via attempting to open a socket)."""
+    if not ZEROCONF_STATE["listener"]:
+        initialize_zeroconf_listener()


This is going to be taken out so that we don't end up with multiple zeroconf listeners.

Instead, the single zeroconf listener will pickle the latest list of instances, and various workers will access the pickled list.

micahscopes · 2019-11-06T23:06:28Z

kolibri/core/discovery/utils/network/search.py

+            continue
+        if not _is_port_open(instance["ip"], instance["port"], timeout=timeout):
+            continue
+        instance["self"] = (


This will get moved into the zeroconf discovery handler.

(and can just be excluded off the bat)

micahscopes · 2019-11-06T23:09:29Z

kolibri/core/discovery/utils/network/search.py

+
+
+def _is_port_open(host, port, timeout=1):
+    with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock:


Add a little local cache so that this check isn't done too frequently for a given peer, but so that the check is done:

immediately following discovery

after a while (~ 20 seconds?)

I'd suggest having it be a "shared" cache (not just local memory), using Django's cache backend, because it might be different web service processes handling the sequential queries.

codecov · 2019-11-13T23:49:40Z

Codecov Report

Merging #6011 into develop will increase coverage by 0.77%.
The diff coverage is 78.52%.

Impacted Files	Coverage Δ
...ageContentPage/SelectNetworkAddressModal/index.vue	`63.63% <0%> (-6.37%)`	⬇️
...ManageContentPage/SelectNetworkAddressModal/api.js	`0% <0%> (ø)`	⬆️
kolibri/core/discovery/api.py	`100% <100%> (ø)`	⬆️
kolibri/plugins/device/assets/src/apiResources.js	`100% <100%> (ø)`	⬆️
kolibri/core/discovery/api_urls.py	`100% <100%> (ø)`	⬆️
kolibri/utils/server.py	`46.24% <25%> (-0.71%)`	⬇️
kolibri/utils/cli.py	`70.55% <45.45%> (-0.88%)`	⬇️
...ge/SelectNetworkAddressModal/SelectAddressForm.vue	`81.33% <74.07%> (-6.39%)`	⬇️
kolibri/core/discovery/utils/network/search.py	`92.85% <92.85%> (ø)`
kolibri/utils/logger.py	`73.43% <0%> (-15.63%)`	⬇️
... and 13 more

.buildkite/upload_artifacts.py

indirectlylit

Awesome thanks @micahscopes

There are a few outstanding notes from @jamalex and I left a few very minor comments also. I'll merge this in and we'll continue the work in subsequent PRs

Also opened this: #6059

indirectlylit · 2019-11-14T16:22:09Z

kolibri/plugins/device/assets/src/views/ManageContentPage/SelectNetworkAddressModal/index.vue

@@ -5,6 +5,7 @@
      v-if="stage === Stages.SELECT_ADDRESS"
      @cancel="resetContentWizardState"
      @click_add_address="goToAddAddress"
+      @click_search_address="goToSearchAddress"


conventionally we use camelCase for events. I see that this matches the others in the component so this is fine, but would be nice to refactor later for consistency

Good catch. I didn't bother to go that in-depth. That code is copied from another file and looking at it now, is going to be deleted anyway.

indirectlylit · 2019-11-14T16:26:22Z

...ns/device/assets/src/views/ManageContentPage/SelectNetworkAddressModal/SelectAddressForm.vue

        />
      </div>
    </template>
+
+    <hr v-if="discoveredAddresses.length > 0">


in subsequent work, let's make sure this is styled as a one-pixel solid line using the theme color $themeTokens.fineLine

indirectlylit · 2019-11-14T16:27:56Z

...ns/device/assets/src/views/ManageContentPage/SelectNetworkAddressModal/SelectAddressForm.vue

+          this.intervalId = clearInterval(this.intervalId);
+        }
+      },
+


same comment as in another PR here: #5631 (comment)

rtibbles · 2019-11-14T16:48:46Z

Woot

jamalex added 15 commits June 15, 2019 14:30

Update _kill_pid to leverage os.kill for Windows as well (Py2.7+)

a543930

(also means that we start with a soft interrupt and only hard-kill later if needed, as we do on Linux)

Utils for broadcasting/scanning zeroconf Kolibri service for discovery

2c3c0cf

Pass the currently specified port through into the services functions.

2922c0f

Register the Kolibri zeroconf service so it will be discoverable

e923e50

Gracefully clean up and unregister Kolibri zeroconf service on shutdown

7d40f1e

Add network search API endpoint for enumerating found Kolibri instances

ffe47c7

Run black and flake8 on files that still needed it to pass CI

bb2d08a

Fix zeroconf github URL in requirements.txt

6648666

Remove .egg-info directories from kolibri/dist as they cause errors

a75293d

Update to Morango 0.4.6

8ae6bbc

Gracefully handle server being started within a thread (e.g. Android)

df7523b

Initial prototype frontend code for zeroconf device discovery.

2881db3

Add base_url to network search results expectations for tests

6dec95c

Put facility name in radio item for device list

7c07682

Update zeroconf to 0.19.3 from PyPI

30e13a8

indirectlylit added the changelog Important user-facing changes label Oct 30, 2019

indirectlylit added this to the 0.13.0 milestone Oct 30, 2019

indirectlylit assigned micahscopes Oct 30, 2019

indirectlylit added the work-in-progress Not ready for review label Oct 30, 2019

indirectlylit mentioned this pull request Oct 30, 2019

Zeroconf-based Kolibri P2P discovery system #5660

Closed

12 tasks

indirectlylit changed the title ~~Content import peer discovery [new]~~ Content import peer discovery Oct 30, 2019

micahscopes reviewed Nov 6, 2019

View reviewed changes

indirectlylit added the TAG: user strings Application text that gets translated label Nov 10, 2019

micahscopes added 2 commits November 12, 2019 17:43

Merge latest develop branch into zeroconf WIP branch

b51c9a0

Rough implementation of latest peer importing designs.

52eed34

micahscopes added 2 commits November 13, 2019 18:45

reset changes made to utils/system.py

a40c1fc

remove debugging line

ae7bb70

micahscopes requested review from jonboiser, nucleogenesis and micahscopes November 14, 2019 00:51

Fixed typo

9e13f66

indirectlylit commented Nov 14, 2019

View reviewed changes

.buildkite/upload_artifacts.py Outdated Show resolved Hide resolved

indirectlylit commented Nov 14, 2019

View reviewed changes

indirectlylit mentioned this pull request Nov 14, 2019

unhandled zeroconf-related errors when shutting down kolibri #6059

Closed

indirectlylit merged commit d179a01 into develop Nov 14, 2019

rtibbles deleted the zeroconf branch November 14, 2019 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Content import peer discovery #6011

Content import peer discovery #6011

indirectlylit commented Oct 30, 2019 •

edited

Loading

micahscopes Nov 6, 2019 •

edited

Loading

micahscopes Nov 6, 2019

jamalex Nov 6, 2019

micahscopes Nov 6, 2019 •

edited

Loading

jamalex Nov 6, 2019

codecov bot commented Nov 13, 2019 •

edited

Loading

indirectlylit left a comment •

edited

Loading

indirectlylit Nov 14, 2019

micahscopes Nov 14, 2019

indirectlylit Nov 14, 2019

indirectlylit Nov 14, 2019

rtibbles commented Nov 14, 2019



		def _is_port_open(host, port, timeout=1):
		with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock:

Content import peer discovery #6011

Content import peer discovery #6011

Conversation

indirectlylit commented Oct 30, 2019 • edited Loading

Summary

Reviewer guidance

References

Contributor Checklist

Reviewer Checklist

micahscopes Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

micahscopes Nov 6, 2019

Choose a reason for hiding this comment

jamalex Nov 6, 2019

Choose a reason for hiding this comment

micahscopes Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

jamalex Nov 6, 2019

Choose a reason for hiding this comment

codecov bot commented Nov 13, 2019 • edited Loading

Codecov Report

indirectlylit left a comment • edited Loading

Choose a reason for hiding this comment

indirectlylit Nov 14, 2019

Choose a reason for hiding this comment

micahscopes Nov 14, 2019

Choose a reason for hiding this comment

indirectlylit Nov 14, 2019

Choose a reason for hiding this comment

indirectlylit Nov 14, 2019

Choose a reason for hiding this comment

rtibbles commented Nov 14, 2019

indirectlylit commented Oct 30, 2019 •

edited

Loading

micahscopes Nov 6, 2019 •

edited

Loading

micahscopes Nov 6, 2019 •

edited

Loading

codecov bot commented Nov 13, 2019 •

edited

Loading

indirectlylit left a comment •

edited

Loading