From 5af2b68e2b6ee2c8693312cca7565012a4232307 Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Wed, 11 Jan 2023 02:45:50 -0300 Subject: [PATCH 1/7] Update plugin notifications for errors and success - Only show notification on plugin load and failure. - In settings page, set current backend status at top of pane instead of showing notification Notices bubbles cluttered the UI while typing updates to settings - Show notification once index updated via settings pane button click There was no notification on index updated, which usually takes time on the backend --- src/interface/obsidian/src/main.ts | 2 +- src/interface/obsidian/src/settings.ts | 26 ++++++++++++++++---------- src/interface/obsidian/src/utils.ts | 9 +++++---- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/interface/obsidian/src/main.ts b/src/interface/obsidian/src/main.ts index 23082895..1725b5ae 100644 --- a/src/interface/obsidian/src/main.ts +++ b/src/interface/obsidian/src/main.ts @@ -45,7 +45,7 @@ export default class Khoj extends Plugin { } async saveSettings() { - await configureKhojBackend(this.settings) + await configureKhojBackend(this.settings, false) .then(() => this.saveData(this.settings)); } } diff --git a/src/interface/obsidian/src/settings.ts b/src/interface/obsidian/src/settings.ts index 74ae9c76..a551942a 100644 --- a/src/interface/obsidian/src/settings.ts +++ b/src/interface/obsidian/src/settings.ts @@ -1,4 +1,4 @@ -import { App, PluginSettingTab, request, Setting } from 'obsidian'; +import { App, Notice, PluginSettingTab, request, Setting } from 'obsidian'; import Khoj from 'src/main'; import { getVaultAbsolutePath } from 'src/utils'; @@ -28,10 +28,8 @@ export class KhojSettingTab extends PluginSettingTab { const { containerEl } = this; containerEl.empty(); - // Add notice if unable to connect to khoj backend - if (!this.plugin.settings.connectedToBackend) { - containerEl.createEl('small', { text: '❗Ensure Khoj backend is running and Khoj URL is correctly set below' }); - } + // Add notice whether able to connect to khoj backend or not + containerEl.createEl('small', { text: this.getBackendStatusMessage() }); // Add khoj settings configurable from the plugin settings tab new Setting(containerEl) @@ -49,8 +47,9 @@ export class KhojSettingTab extends PluginSettingTab { .addText(text => text .setValue(`${this.plugin.settings.khojUrl}`) .onChange(async (value) => { - this.plugin.settings.khojUrl = value; - await this.plugin.saveSettings(); + this.plugin.settings.khojUrl = value.trim(); + await this.plugin.saveSettings() + .finally(() => containerEl.firstElementChild?.setText(this.getBackendStatusMessage())); })); new Setting(containerEl) .setName('Results Count') @@ -69,8 +68,15 @@ export class KhojSettingTab extends PluginSettingTab { .setButtonText('Update') .setCta() .onClick(async () => { - await request(`${this.plugin.settings.khojUrl}/api/update?t=markdown&force=true`); - } - )); + await request(`${this.plugin.settings.khojUrl}/api/update?t=markdown&force=true`) + .then(() => new Notice('✅ Updated Khoj index.')) + }) + ); + } + + getBackendStatusMessage() { + return !this.plugin.settings.connectedToBackend + ? '❗Disconnected from Khoj backend. Ensure Khoj backend is running and Khoj URL is correctly set below.' + : '✅ Connected to Khoj backend.'; } } diff --git a/src/interface/obsidian/src/utils.ts b/src/interface/obsidian/src/utils.ts index 3d4220b2..10d08b3e 100644 --- a/src/interface/obsidian/src/utils.ts +++ b/src/interface/obsidian/src/utils.ts @@ -9,7 +9,7 @@ export function getVaultAbsolutePath(): string { return ''; } -export async function configureKhojBackend(setting: KhojSetting) { +export async function configureKhojBackend(setting: KhojSetting, notify: boolean = true) { let mdInVault = `${setting.obsidianVaultPath}/**/*.md`; let khojConfigUrl = `${setting.khojUrl}/api/config/data`; @@ -21,7 +21,8 @@ export async function configureKhojBackend(setting: KhojSetting) { }) .catch(error => { setting.connectedToBackend = false; - new Notice(`❗️Ensure Khoj backend is running and Khoj URL is pointing to it in the plugin settings.\n\n${error}`); + if (notify) + new Notice(`❗️Ensure Khoj backend is running and Khoj URL is pointing to it in the plugin settings.\n\n${error}`); }) // Short-circuit configuring khoj if unable to connect to khoj backend if (!setting.connectedToBackend) return; @@ -79,10 +80,10 @@ export async function configureKhojBackend(setting: KhojSetting) { updateKhojBackend(setting.khojUrl, data); console.log(`Khoj: Updated markdown config in khoj backend config:\n${JSON.stringify(data["content-type"]["markdown"])}`) } - new Notice(`✅ Successfully Setup Khoj`); }) .catch(error => { - new Notice(`❗️Failed to configure Khoj backend. Contact developer on Github.\n\nError: ${error}`); + if (notify) + new Notice(`❗️Failed to configure Khoj backend. Contact developer on Github.\n\nError: ${error}`); }) } From 86a1e4360599060be31879da3b3d24961367c81b Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Wed, 11 Jan 2023 03:01:09 -0300 Subject: [PATCH 2/7] Return HTTP Exception on /api/update API call failure - Previously the backend was just throwing backend error. The frontend calling the /update API wasn't getting notified - Now the frontend can react appropriately and make the issue visible to the user --- src/routers/api.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/routers/api.py b/src/routers/api.py index 0da30eb3..9480c914 100644 --- a/src/routers/api.py +++ b/src/routers/api.py @@ -1,11 +1,11 @@ # Standard Packages import yaml -import time import logging from typing import Optional # External Packages from fastapi import APIRouter +from fastapi import HTTPException # Internal Packages from src.configure import configure_processor, configure_search @@ -114,12 +114,22 @@ def search(q: str, n: Optional[int] = 5, t: Optional[SearchType] = None, r: Opti @api.get('/update') def update(t: Optional[SearchType] = None, force: Optional[bool] = False): - state.search_index_lock.acquire() - state.model = configure_search(state.model, state.config, regenerate=force, t=t) - state.search_index_lock.release() - logger.info("Search Index updated via API call") + try: + state.search_index_lock.acquire() + state.model = configure_search(state.model, state.config, regenerate=force, t=t) + state.search_index_lock.release() + except ValueError as e: + logger.error(e) + raise HTTPException(status_code=500, detail=str(e)) + else: + logger.info("Search Index updated via API call") - state.processor_config = configure_processor(state.config.processor) - logger.info("Processor reconfigured via API call") + try: + state.processor_config = configure_processor(state.config.processor) + except ValueError as e: + logger.error(e) + raise HTTPException(status_code=500, detail=str(e)) + else: + logger.info("Processor reconfigured via API call") return {'status': 'ok', 'message': 'khoj reloaded'} From 4407e23c197622878ac20c2677cad6b3499cef03 Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Wed, 11 Jan 2023 03:03:22 -0300 Subject: [PATCH 3/7] Only index current vault on Khoj. Remove plugin setting to configure it - Overview Limits using Khoj with a single vault at a time. This is automatically configured to the most recently opened vault. Once directory filters are supported on backend, the plugin will be updated to index multiple vault but search only current vault from current vaults khoj obsidian plugin - Code Details - Remove setting to configure Vault directory from Khoj Obsidian plugin - Automatically configure Khoj to index only current Vault. - Overwrites any previous vaults that were intended to be indexed by Khoj backend - Force update of index after configuring vault - Why It's not helpful for now and can lead to more problems, confusion. Once directory filters --- src/interface/obsidian/src/settings.ts | 12 ------------ src/interface/obsidian/src/utils.ts | 8 ++++---- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/interface/obsidian/src/settings.ts b/src/interface/obsidian/src/settings.ts index a551942a..f2ce5a56 100644 --- a/src/interface/obsidian/src/settings.ts +++ b/src/interface/obsidian/src/settings.ts @@ -1,18 +1,15 @@ import { App, Notice, PluginSettingTab, request, Setting } from 'obsidian'; import Khoj from 'src/main'; -import { getVaultAbsolutePath } from 'src/utils'; export interface KhojSetting { resultsCount: number; khojUrl: string; - obsidianVaultPath: string; connectedToBackend: boolean; } export const DEFAULT_SETTINGS: KhojSetting = { resultsCount: 6, khojUrl: 'http://localhost:8000', - obsidianVaultPath: getVaultAbsolutePath(), connectedToBackend: false, } @@ -32,15 +29,6 @@ export class KhojSettingTab extends PluginSettingTab { containerEl.createEl('small', { text: this.getBackendStatusMessage() }); // Add khoj settings configurable from the plugin settings tab - new Setting(containerEl) - .setName('Vault Path') - .setDesc('The Obsidian Vault to search with Khoj') - .addText(text => text - .setValue(`${this.plugin.settings.obsidianVaultPath}`) - .onChange(async (value) => { - this.plugin.settings.obsidianVaultPath = value; - await this.plugin.saveSettings(); - })); new Setting(containerEl) .setName('Khoj URL') .setDesc('The URL of the Khoj backend') diff --git a/src/interface/obsidian/src/utils.ts b/src/interface/obsidian/src/utils.ts index 10d08b3e..7c1cf7df 100644 --- a/src/interface/obsidian/src/utils.ts +++ b/src/interface/obsidian/src/utils.ts @@ -10,7 +10,7 @@ export function getVaultAbsolutePath(): string { } export async function configureKhojBackend(setting: KhojSetting, notify: boolean = true) { - let mdInVault = `${setting.obsidianVaultPath}/**/*.md`; + let mdInVault = `${getVaultAbsolutePath()}/**/*.md`; let khojConfigUrl = `${setting.khojUrl}/api/config/data`; // Check if khoj backend is configured, show error if backend is not running @@ -34,7 +34,7 @@ export async function configureKhojBackend(setting: KhojSetting, notify: boolean // If khoj backend not configured yet if (!khoj_already_configured) { // Create khoj content-type config with only markdown configured - let khojObsidianPluginPath = `${setting.obsidianVaultPath}/${this.app.vault.configDir}/plugins/khoj/`; + let khojObsidianPluginPath = `${getVaultAbsolutePath()}/${this.app.vault.configDir}/plugins/khoj/`; data["content-type"] = { "markdown": { "input-filter": [mdInVault], @@ -55,7 +55,7 @@ export async function configureKhojBackend(setting: KhojSetting, notify: boolean else if (!data["content-type"]["markdown"]) { // Add markdown config to khoj content-type config // Set markdown config to index markdown files in configured obsidian vault - let khojObsidianPluginPath = `${setting.obsidianVaultPath}/${this.app.vault.configDir}/plugins/khoj/`; + let khojObsidianPluginPath = `${getVaultAbsolutePath()}/${this.app.vault.configDir}/plugins/khoj/`; data["content-type"]["markdown"] = { "input-filter": [mdInVault], "input-files": null, @@ -99,5 +99,5 @@ export async function updateKhojBackend(khojUrl: string, khojConfig: Object) { // Save khojConfig on khoj backend at khojConfigUrl await request(requestContent) // Refresh khoj search index after updating config - .then(_ => request(`${khojUrl}/api/update?t=markdown`)); + .then(_ => request(`${khojUrl}/api/update?t=markdown&force=true`)); } From 513c86c6a145934f9d19d81c051639d044537fca Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Wed, 11 Jan 2023 13:34:15 -0300 Subject: [PATCH 4/7] Set index file paths relative to current or default path on khoj backend We need the index file paths to make sense on the khoj backend server Having path of index on backend relative to current vault directory on frontend ignores the fact that the frontend maybe on a different machine than the khoj backend server Using unique index name per vault allows switching vaults without overwriting indices of other vaults created on khoj backend when khoj obsidian plugin is loaded on opening a different vault --- src/interface/obsidian/src/utils.ts | 33 ++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/interface/obsidian/src/utils.ts b/src/interface/obsidian/src/utils.ts index 7c1cf7df..377297f1 100644 --- a/src/interface/obsidian/src/utils.ts +++ b/src/interface/obsidian/src/utils.ts @@ -13,7 +13,7 @@ export async function configureKhojBackend(setting: KhojSetting, notify: boolean let mdInVault = `${getVaultAbsolutePath()}/**/*.md`; let khojConfigUrl = `${setting.khojUrl}/api/config/data`; - // Check if khoj backend is configured, show error if backend is not running + // Check if khoj backend is configured, note if cannot connect to backend let khoj_already_configured = await request(khojConfigUrl) .then(response => { setting.connectedToBackend = true; @@ -27,6 +27,13 @@ export async function configureKhojBackend(setting: KhojSetting, notify: boolean // Short-circuit configuring khoj if unable to connect to khoj backend if (!setting.connectedToBackend) return; + // Set index name from the path of the current vault + let indexName = getVaultAbsolutePath().replace(/\//g, '_').replace(/ /g, '_'); + // Get default index directory from khoj backend + let khojDefaultIndexDirectory = await request(`${khojConfigUrl}/default`) + .then(response => JSON.parse(response)) + .then(data => { return getIndexDirectoryFromBackendConfig(data); }); + // Get current config if khoj backend configured, else get default config from khoj backend await request(khoj_already_configured ? khojConfigUrl : `${khojConfigUrl}/default`) .then(response => JSON.parse(response)) @@ -34,13 +41,12 @@ export async function configureKhojBackend(setting: KhojSetting, notify: boolean // If khoj backend not configured yet if (!khoj_already_configured) { // Create khoj content-type config with only markdown configured - let khojObsidianPluginPath = `${getVaultAbsolutePath()}/${this.app.vault.configDir}/plugins/khoj/`; data["content-type"] = { "markdown": { "input-filter": [mdInVault], "input-files": null, - "embeddings-file": `${khojObsidianPluginPath}/markdown_embeddings.pt`, - "compressed-jsonl": `${khojObsidianPluginPath}/markdown.jsonl.gz`, + "embeddings-file": `${khojDefaultIndexDirectory}/${indexName}.pt`, + "compressed-jsonl": `${khojDefaultIndexDirectory}/${indexName}.jsonl.gz`, } } // Disable khoj processors, as not required @@ -55,12 +61,11 @@ export async function configureKhojBackend(setting: KhojSetting, notify: boolean else if (!data["content-type"]["markdown"]) { // Add markdown config to khoj content-type config // Set markdown config to index markdown files in configured obsidian vault - let khojObsidianPluginPath = `${getVaultAbsolutePath()}/${this.app.vault.configDir}/plugins/khoj/`; data["content-type"]["markdown"] = { "input-filter": [mdInVault], "input-files": null, - "embeddings-file": `${khojObsidianPluginPath}/markdown_embeddings.pt`, - "compressed-jsonl": `${khojObsidianPluginPath}/markdown.jsonl.gz`, + "embeddings-file": `${khojDefaultIndexDirectory}/${indexName}.pt`, + "compressed-jsonl": `${khojDefaultIndexDirectory}/${indexName}.jsonl.gz`, } // Save updated config and refresh index on khoj backend @@ -73,9 +78,13 @@ export async function configureKhojBackend(setting: KhojSetting, notify: boolean data["content-type"]["markdown"]["input-filter"][0] !== mdInVault) { // Update markdown config in khoj content-type config // Set markdown config to only index markdown files in configured obsidian vault - data["content-type"]["markdown"]["input-filter"] = [mdInVault] - data["content-type"]["markdown"]["input-files"] = null - + let khojIndexDirectory = getIndexDirectoryFromBackendConfig(data); + data["content-type"]["markdown"] = { + "input-filter": [mdInVault], + "input-files": null, + "embeddings-file": `${khojIndexDirectory}/${indexName}.pt`, + "compressed-jsonl": `${khojIndexDirectory}/${indexName}.jsonl.gz`, + } // Save updated config and refresh index on khoj backend updateKhojBackend(setting.khojUrl, data); console.log(`Khoj: Updated markdown config in khoj backend config:\n${JSON.stringify(data["content-type"]["markdown"])}`) @@ -101,3 +110,7 @@ export async function updateKhojBackend(khojUrl: string, khojConfig: Object) { // Refresh khoj search index after updating config .then(_ => request(`${khojUrl}/api/update?t=markdown&force=true`)); } + +function getIndexDirectoryFromBackendConfig(khojConfig: any) { + return khojConfig["content-type"]["markdown"]["embeddings-file"].split("/").slice(0, -1).join("/"); +} \ No newline at end of file From 4e1abd1b729073e68b5f159ea567f183dc1bfa12 Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Wed, 11 Jan 2023 13:43:43 -0300 Subject: [PATCH 5/7] Disable update button while indexing vault in plugin settings --- src/interface/obsidian/src/settings.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/interface/obsidian/src/settings.ts b/src/interface/obsidian/src/settings.ts index f2ce5a56..e3486c81 100644 --- a/src/interface/obsidian/src/settings.ts +++ b/src/interface/obsidian/src/settings.ts @@ -49,15 +49,26 @@ export class KhojSettingTab extends PluginSettingTab { this.plugin.settings.resultsCount = parseInt(value); await this.plugin.saveSettings(); })); - new Setting(containerEl) + let indexVaultSetting = new Setting(containerEl); + indexVaultSetting .setName('Index Vault') .setDesc('Manually force Khoj to re-index your Obsidian Vault') .addButton(button => button .setButtonText('Update') .setCta() .onClick(async () => { + // Disable button while updating index + button.setButtonText('Updating...'); + button.removeCta() + indexVaultSetting = indexVaultSetting.setDisabled(true); + await request(`${this.plugin.settings.khojUrl}/api/update?t=markdown&force=true`) - .then(() => new Notice('✅ Updated Khoj index.')) + .then(() => new Notice('✅ Updated Khoj index.')); + + // Re-enable button once index is updated + button.setButtonText('Update'); + button.setCta() + indexVaultSetting = indexVaultSetting.setDisabled(false); }) ); } From 1c813a68842f20dcecd48a53b9202eab72baf68e Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Wed, 11 Jan 2023 14:05:46 -0300 Subject: [PATCH 6/7] Convert results count setting to slider in plugin settings pane --- src/interface/obsidian/src/settings.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/interface/obsidian/src/settings.ts b/src/interface/obsidian/src/settings.ts index e3486c81..edfbb123 100644 --- a/src/interface/obsidian/src/settings.ts +++ b/src/interface/obsidian/src/settings.ts @@ -42,11 +42,12 @@ export class KhojSettingTab extends PluginSettingTab { new Setting(containerEl) .setName('Results Count') .setDesc('The number of search results to show') - .addText(text => text - .setPlaceholder('6') - .setValue(`${this.plugin.settings.resultsCount}`) + .addSlider(slider => slider + .setLimits(1, 10, 1) + .setValue(this.plugin.settings.resultsCount) + .setDynamicTooltip() .onChange(async (value) => { - this.plugin.settings.resultsCount = parseInt(value); + this.plugin.settings.resultsCount = value; await this.plugin.saveSettings(); })); let indexVaultSetting = new Setting(containerEl); From 123b077c688aa107cc147691b1a33cae7ac70b73 Mon Sep 17 00:00:00 2001 From: Debanjum Singh Solanky Date: Wed, 11 Jan 2023 16:51:16 -0300 Subject: [PATCH 7/7] Use apt update before apt install in test workflow on Github --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8c712dc0..2902a56f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,7 +34,7 @@ jobs: - name: Install Dependencies run: | - sudo apt install libegl1 -y + sudo apt update && sudo apt install -y libegl1 python -m pip install --upgrade pip pip install pytest