-
Notifications
You must be signed in to change notification settings - Fork 35
clean up live view #34
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
clean up live view #34
Conversation
- cleared out all bookmarks in the bookmark bar - disabled the bookmark bar in Preferences
6562d59 to
43d739d
Compare
Sayan-
left a comment
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.
Lotta wins in this! None of my comments are blocking.
Confirming I didn't review user-data
| name=chromium-headful-test | ||
|
|
||
| # Name for the Kraft Cloud volume that will carry Chromium flags | ||
| volume_name="${name}-flags" |
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.
nit - the mix of upper and lower case vars is a little confusing
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.
I think the convention that lives in my head is: lowercase for local vars, UPPERCASE for things related to env vars
not super consistent but can nudge things in that direction
| # Re-create the volume from scratch every run | ||
| kraft cloud volume rm "$volume_name" || true | ||
| kraft cloud volume create -n "$volume_name" -s 16M | ||
| # Import the flags directory into the freshly created volume | ||
| kraft cloud volume import -s "$FLAGS_DIR" -v "$volume_name" |
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.
do volume names need to be unique? mainly concerned about stepping on each others toes
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.
yeah... tricky part is ensuring we delete the previously-created volume (e.g. not sure how to kraft cloud volume rm "$volume_name" || true if we make volume name unique on every run)
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.
yeah really good point - honestly this is where I usually just toss $USER in but that's not necessarily desirable 😅
| oapi "github.com/onkernel/kernel-images/server/lib/oapi" | ||
| ) | ||
|
|
||
| func (s *ApiService) MoveMouse(ctx context.Context, request oapi.MoveMouseRequestObject) (oapi.MoveMouseResponseObject, error) { |
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.
non-blocking. there is a local dev flow for the server. since xdotool isn't supported on macos we may want to detect and return a 501 instead
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.
my local dev flow was building / running the docker image and curl'ing this endpoint... wasn't too bad
on mac os we might be able to do this with applescript
as always things get complicated going from docker -> unikraft - disable scale to zero - add a sleep because this makes it work
| # wait... not sure but this just increases the likelihood of success | ||
| sleep 5 |
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.
what were the failure modes you were running into?
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.
without the sleep I'd often open the live view and see the mouse hovering over the "X" to dismiss the warning, suggesting that xdotool did it's thing before the warning or chromium appeared
will add to the comment describing this
images/chromium-headful/wrapper.sh
Outdated
| echo "Waiting for kernel-images API port 127.0.0.1:10001..." | ||
| while ! nc -z 127.0.0.1 10001 2>/dev/null; do |
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.
potential optimization: I'd expect the server to come up faster than chromium so we could probably trim some time by inverting the order
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.
👍
|
|
||
| kraft pkg \ | ||
| --name index.unikraft.io/$image \ | ||
| --name $UKC_INDEX/$IMAGE \ |
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.
should we default to the public unikraft index to make this easier for folks?
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.
👍
live view has much more chill now:
w=1024&h=1336&fr=60readOnly=trueOther nice things:
recommend reviewing commit by commit or just ignoring everything in user-data/. In order to load custom preferences had to grab the default user-data directory that chromium creates and modify it / load it onto the image