Skip to content
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

[cli]] updates for multi-os browser for langstream UI #682

Merged
merged 25 commits into from
Nov 4, 2023

Conversation

mark878
Copy link
Contributor

@mark878 mark878 commented Nov 2, 2023

main issue was to figure out that the thread needs to wait a bit after processBuilder is called to start the browser and waitfor() can't be used because it doesn't return until the started process completes.

I only tested this out is WSL. Needs to be tested on standard linux, macos and windows.

Added logic to determine OS and spawn a browser process based on the OS

@mark878
Copy link
Contributor Author

mark878 commented Nov 2, 2023

updated the test(s)

Copy link
Member

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work !

Copy link
Member

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eolivelli eolivelli changed the title updates for multi-os browser for langstream UI [cli]] updates for multi-os browser for langstream UI Nov 4, 2023
@eolivelli eolivelli merged commit 6ca69e9 into LangStream:main Nov 4, 2023
10 checks passed
@mark878 mark878 deleted the browser branch November 4, 2023 16:59
benfrank241 pushed a commit to vectorize-io/langstream that referenced this pull request May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants