Skip to content

Commit fad778d

Browse files
rtibblesclaude
andcommitted
Address remaining PR review feedback
- Make KolibriEnvironmentSetup.initialized volatile for cross-thread visibility - Use LC_CTYPE instead of LC_ALL to avoid overriding user locale for non-encoding operations - Remove commented-out server service code from App.onCreate() - Use context.getApplicationContext() in WorkController constructor to prevent potential Activity context leaks - Redact auth token from log output in AppPlugin.SERVING - Use explicit "kolibri:job:" tag prefix for WorkManager tasks instead of fragile negative pattern matching against class name prefixes - Pin fkirc/skip-duplicate-actions to v5.3.1 instead of mutable @master Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 653f030 commit fad778d

8 files changed

Lines changed: 15 additions & 17 deletions

File tree

.github/workflows/build_and_test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
should_skip: ${{ steps.skip_check.outputs.should_skip }}
1717
steps:
1818
- id: skip_check
19-
uses: fkirc/skip-duplicate-actions@master
19+
uses: fkirc/skip-duplicate-actions@v5.3.1
2020
with:
2121
github_token: ${{ github.token }}
2222
paths_ignore: '["**.po", "**.json"]'

.github/workflows/pre-commit.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ jobs:
1717
should_skip: ${{ steps.skip_check.outputs.should_skip }}
1818
steps:
1919
- id: skip_check
20-
uses: fkirc/skip-duplicate-actions@master
20+
uses: fkirc/skip-duplicate-actions@v5.3.1
2121
with:
2222
github_token: ${{ github.token }}
2323
paths_ignore: '["**.po", "**.json"]'

app/src/main/java/org/learningequality/Kolibri/App.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ public void onCreate() {
3030
// Initialize Kolibri environment
3131
KolibriEnvironmentSetup.initializeEnv(this);
3232

33-
// Start Kolibri server service
34-
// Intent serviceIntent = new Intent(this, KolibriServerService.class);
35-
// startService(serviceIntent);
36-
3733
createNotificationChannels();
3834
// Register activity lifecycle callbacks
3935
registerActivityLifecycleCallbacks(new KolibriActivityLifecycleCallbacks());

app/src/main/java/org/learningequality/Kolibri/KolibriEnvironmentSetup.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
/** Sets up the Kolibri environment including environment variables and Python initialization */
1414
public class KolibriEnvironmentSetup {
1515
private static final String TAG = "KolibriEnvironmentSetup";
16-
private static boolean initialized = false;
16+
private static volatile boolean initialized = false;
1717

1818
/** Initialize Kolibri environment Starts Python and sets up all required environment variables */
1919
public static synchronized void initializeEnv(Context context) {
@@ -81,7 +81,7 @@ private static void setPythonEnvironmentVariables(Context context) {
8181
environ.callAttr("__setitem__", "TZ", tz.getID());
8282

8383
// Locale
84-
environ.callAttr("__setitem__", "LC_ALL", "en_US.UTF-8");
84+
environ.callAttr("__setitem__", "LC_CTYPE", "en_US.UTF-8");
8585

8686
// Android language
8787
String language = context.getResources().getConfiguration().getLocales().get(0).getLanguage();

app/src/main/java/org/learningequality/Kolibri/WorkController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public class WorkController {
2727
private final AtomicBoolean isConnected = new AtomicBoolean(false);
2828

2929
private WorkController(Context context) {
30-
this.context = context;
30+
this.context = context.getApplicationContext();
3131
this.connected = new CompletableFuture<>();
3232
}
3333

app/src/main/java/org/learningequality/Kolibri/task/Task.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ public static String enqueueOnce(
5555

5656
// Build work request with delay and priority
5757
OneTimeWorkRequest.Builder requestBuilder =
58-
new OneTimeWorkRequest.Builder(workerClass).setInputData(inputData).addTag(id);
58+
new OneTimeWorkRequest.Builder(workerClass)
59+
.setInputData(inputData)
60+
.addTag("kolibri:job:" + id);
5961

6062
// Set initial delay if specified
6163
if (delay > 0) {
@@ -108,7 +110,7 @@ public static void clear(String id) {
108110
try {
109111
Context context = ContextUtil.getApplicationContext();
110112
RemoteWorkManager workManager = RemoteWorkManager.getInstance(context);
111-
workManager.cancelAllWorkByTag(id);
113+
workManager.cancelAllWorkByTag("kolibri:job:" + id);
112114
Log.d(TAG, "Cancelled task " + id);
113115
} catch (Exception e) {
114116
Log.e(TAG, "Failed to cancel task " + id, e);

app/src/main/python/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def SERVING(self, port):
5454
port=port
5555
) + interface.get_initialize_url(auth_token=auth_token)
5656

57-
logger.info(f"Kolibri server ready at: {init_url}")
57+
logger.info("Kolibri server ready on port %s", port)
5858

5959
# Signal Java that server is ready
6060
KolibriServerViewModel.getInstance().setServerReady(True, port, init_url)

app/src/main/python/task_reconciler.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ def _get_workmanager_job_ids():
6363
work_query = WorkQuery.fromStates(states)
6464
work_info_list = work_manager.getWorkInfos(work_query).get()
6565

66-
# Extract job IDs from tags
66+
# Extract Kolibri job IDs from tags
67+
# Tags include both worker class FQNs and our job ID tag set via Task.enqueueOnce
68+
# We use an explicit prefix to identify our tags rather than excluding known patterns
6769
job_ids = set()
6870
for work_info in work_info_list.toArray():
6971
tags = work_info.getTags()
7072
for tag in tags.toArray():
71-
# Filter out class names (worker class FQNs), keep job IDs
72-
# Job IDs can be UUIDs, numbers, or strings like "streamed_cache_cleanup"
73-
if not tag.startswith("org.") and not tag.startswith("androidx."):
74-
job_ids.add(tag)
73+
if tag.startswith("kolibri:job:"):
74+
job_ids.add(tag[len("kolibri:job:") :])
7575

7676
return job_ids
7777

0 commit comments

Comments
 (0)