-
Notifications
You must be signed in to change notification settings - Fork 58
added Try Valkey support #255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Shai Zarka <[email protected]>
Is this a draft or ready to review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces the "Try Valkey" feature, enabling users to interact with Valkey‑CLI commands via a browser-based virtualized environment powered by v86.
- Implements a new HTML template (valkey-try-me.html) that sets up and configures the virtual machine, including binary caching and emulator lifecycle management.
- Adds a navigation link to the Try Valkey page in the default template.
- Provides a minimal Markdown file to route the Try Valkey page.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
templates/valkey-try-me.html | New template for the in-browser emulator, including caching and VM controls |
templates/default.html | Added navigation link for Try Valkey |
content/try-valkey/index.md | Added frontmatter to set up the Try Valkey page |
const decompressedData = pako.ungzip(new Uint8Array(xhr.response)); | ||
callback(decompressedData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding error handling around pako.ungzip to catch decompression errors and provide a user-friendly error message.
const decompressedData = pako.ungzip(new Uint8Array(xhr.response)); | |
callback(decompressedData); | |
try { | |
const decompressedData = pako.ungzip(new Uint8Array(xhr.response)); | |
callback(decompressedData); | |
} catch (error) { | |
console.error("Decompression error:", error); | |
document.getElementById("progressText").innerText = "Decompression failed. Please try again."; | |
} |
Copilot uses AI. Check for mistakes.
["mousemove", "keydown", "touchstart"].forEach(event => { | ||
window.addEventListener(event, resetInactivityTimer); | ||
}); | ||
|
||
serialAdapter.term.onKey(() => resetInactivityTimer()); // Typing | ||
serialAdapter.term.onData(() => resetInactivityTimer()); // Sending data | ||
serialAdapter.term.onCursorMove(() => resetInactivityTimer()); // Mouse activity | ||
}; | ||
} | ||
|
||
let inactivityTimeout; | ||
const INACTIVITY_LIMIT = 60*1000*10 //inactivity limit is 10 minutes | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider debouncing the calls to resetInactivityTimer to reduce potential performance overhead during rapid key events.
["mousemove", "keydown", "touchstart"].forEach(event => { | |
window.addEventListener(event, resetInactivityTimer); | |
}); | |
serialAdapter.term.onKey(() => resetInactivityTimer()); // Typing | |
serialAdapter.term.onData(() => resetInactivityTimer()); // Sending data | |
serialAdapter.term.onCursorMove(() => resetInactivityTimer()); // Mouse activity | |
}; | |
} | |
let inactivityTimeout; | |
const INACTIVITY_LIMIT = 60*1000*10 //inactivity limit is 10 minutes | |
const debouncedResetInactivityTimer = debounce(resetInactivityTimer, 200); | |
["mousemove", "keydown", "touchstart"].forEach(event => { | |
window.addEventListener(event, debouncedResetInactivityTimer); | |
}); | |
serialAdapter.term.onKey(() => debouncedResetInactivityTimer()); // Typing | |
serialAdapter.term.onData(() => debouncedResetInactivityTimer()); // Sending data | |
serialAdapter.term.onCursorMove(() => debouncedResetInactivityTimer()); // Mouse activity | |
}; | |
} | |
let inactivityTimeout; | |
const INACTIVITY_LIMIT = 60*1000*10 //inactivity limit is 10 minutes | |
function debounce(func, wait) { | |
let timeout; | |
return function(...args) { | |
clearTimeout(timeout); | |
timeout = setTimeout(() => func.apply(this, args), wait); | |
}; | |
} |
Copilot uses AI. Check for mistakes.
const LAST_MODIFIED_KEY = "valkey_last_modified"; | ||
let emulator; | ||
|
||
// Open or create IndexedDB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider adding more detailed comments explaining the structure and purpose of the IndexedDB caching logic to improve maintainability.
// Open or create IndexedDB | |
/** | |
* Opens an IndexedDB database named "binaryCacheDB" or creates it if it doesn't exist. | |
* This database is used to cache the binary data for the emulator, reducing the need | |
* to repeatedly download the binary file. The database contains a single object store | |
* named "cache" with a keyPath of "key". | |
* | |
* @returns {Promise<IDBDatabase>} A promise that resolves to the opened database instance. | |
*/ |
Copilot uses AI. Check for mistakes.
@madolson currently the content is pointing a test bucket with preloaded binaries. I think the PR should be reviewed after @zarkash-aws will change it to point to our production bucket |
Signed-off-by: Shai Zarka <[email protected]>
Signed-off-by: Shai Zarka <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
over all the PR LGTM
@zarkash-aws Can you please render this PR and attach the screen shots to this PR top comment.
<script src="https://download.valkey.io/try-me-valkey/vos/v86/libv86.js"></script> | ||
<script src="https://download.valkey.io/try-me-valkey/vos/xterm/xterm.min.js"></script> | ||
<script src="https://download.valkey.io/try-me-valkey/vos/pako/pako.min.js"></script> | ||
<script src="https://download.valkey.io/try-me-valkey/vos/v86/serial_xterm.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CDN usage looks good to me
@stockholmux @ranshid Do either of you two want to look it over? I didn't really look into the JS code that closely. |
Signed-off-by: Madelyn Olson <[email protected]>
General Description
This PR implements a browser-based environment that allows users to interact with Valkey-CLI commands without needing local installation. This feature was initially introduced in Issue #1412, and it aims to provide a lightweight, easy-to-access platform for users to explore Valkey’s functionalities directly in the browser.
Implementation
The solution uses a virtualized environment powered by v86, a tool that emulates an x86-compatible CPU and hardware. The machine code for the environment is translated into WebAssembly modules at runtime, allowing it to run entirely within the browser.
Serving of Binary Files
The virtual machine used for this setup relies on two main binary file components:
These binaries are served from an S3 bucket (currently set as a test bucket) via CloudFront CDN.
New Image Creation Process
Currently, the creation of new images for future Valkey versions will be done manually. However, we plan to automate this process in the future to simplify updates as new versions of Valkey are released.
I have also created a repository for documentation and for creating new images for Try Valkey.
Screenshots
new "Try Valkey" tab



data warning
after pressing "load emulator"