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

Docker support #106

Merged
merged 7 commits into from
Feb 12, 2023
Merged

Docker support #106

merged 7 commits into from
Feb 12, 2023

Conversation

na4ma4
Copy link

@na4ma4 na4ma4 commented Jan 30, 2023

This probably needs some review since the scope creeped a bit.

This PR contains:

  • devcontainer support for vscode.
  • Dockerfile for building.
  • GitHub workflows for build and push.
  • changes to configuration and view command to handle command line parameters.
  • basic .pylintrc config (will need to be updated for your specified code-style).

I've added a DOCKER_CONTAINER environment variable to the container, which causes the config.submit() to override with sane values (bind_host: 0.0.0.0, open_in_webbrowser: False), so when using the container users don't need to specify the bind host or not to attempt to open a webbrowser.

Python isn't my daily driver language, so it feels a bit hacky, I can split this up into multiple PRs (for devcontainer stuff, etc), but I'm expecting this to only be the first draft.

This PR addresses issue #81

@Owez
Copy link
Owner

Owez commented Jan 30, 2023

Yeah sorry about that, a lot of major restructuring has been happening recently.

With #105 being the way forward with Yark, I'm not sure how to fit docker into the architecture. I want people to be able to self-host their own archives so multiple trusted people can access them, so maybe the new GUI will have a "remote connect" URL and docker will just contain a headless API server like what the GUI normally connects to (federated archive servers).

I'll review later today

@Owez Owez self-assigned this Jan 30, 2023
@Owez Owez added the enhancement New feature or request label Jan 30, 2023
Copy link
Owner

@Owez Owez left a comment

Choose a reason for hiding this comment

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

Good docker integration here, just some comments and general structure changes to do in the cli file

.github/workflows/docker.yml Outdated Show resolved Hide resolved
.github/workflows/docker.yml Outdated Show resolved Hide resolved
yark/archiver/config.py Outdated Show resolved Hide resolved
yark/archiver/config.py Outdated Show resolved Hide resolved
yark/archiver/config.py Show resolved Hide resolved
yark/cli.py Outdated Show resolved Hide resolved
yark/cli.py Outdated Show resolved Hide resolved
yark/cli.py Outdated Show resolved Hide resolved
@Owez Owez marked this pull request as ready for review January 30, 2023 14:11
- moved webbrowser opening function to config
- renamed `--disable-webbrowser` to `--headless`
- added docstrings and comments
- updated help output
@na4ma4
Copy link
Author

na4ma4 commented Feb 1, 2023

With future being yark server/yark gui separated and federated yark servers, most of these changes are probably just short term patches until it is ready.

@Owez
Copy link
Owner

Owez commented Feb 1, 2023

With future being yark server/yark gui separated and federated yark servers, most of these changes are probably just short term patches until it is ready.

Yep, I'll redo everything thats needed once the project is restructured. My current thinking is:

  • yark-pages: GUI project which connects to yark-api for data/management
  • yark-api: REST API for managing and viewing archives which uses yark as a dependency
    • This is what'll be in docker
    • Also packaged inside of yark-pages so people can do local archives
  • yark: Core library
  • yark-cli: Quick n' dirty wrapper around yark for basic archive management, not as important as the GUI

@na4ma4
Copy link
Author

na4ma4 commented Feb 2, 2023

It makes sense for yark-pages and yark-api to be bundled with an option to disable one or the other.

So you could start a docker container that has a volume and runs the yark-api but also has yark-pages for management.
or start a container with a volume just for yark-api and either ignore that yark-pages is bundled or disable it with config.
or start a purely ephmeral container that only has yark-pages for hosting the interface.

How dynamic is yark-pages, could it be hosted as a static site, or does it need to be told where the yark-api is (if not together) ?

@na4ma4
Copy link
Author

na4ma4 commented Feb 2, 2023

when this PR is ready I'll rebase it before you merge it.

@Owez
Copy link
Owner

Owez commented Feb 2, 2023

Good point, it would be really nice to download that single binary and be able to run the api then and there; makes sense for ease of use and for simplified CI/CD on Yark's side. It looks like there is decent sidecaring available for it. I do want Docker deployments to be the main way that people deploy headless archives for remote connections going forward.

I think yark-pages is for now limited to being a desktop application. I ideally wanted yark-pages to be a full web-app and it could do that because it's basically a static SvelteKit (think React/Angular) app put inside of the fancy tauri webview. Unfortunately tauri is in an early state and it makes it so yark-pages has to use some of its native api for file stuff which is hard to seperate.

TL;DR:

  • bundle -pages and -api together
  • api can be ran headless from the bundle
  • -pages can't support web at the moment
  • docker is good

@na4ma4
Copy link
Author

na4ma4 commented Feb 2, 2023

Why does yark-pages need to do file stuff though, wouldn't any file stuff be the job of the API ?

@Owez
Copy link
Owner

Owez commented Feb 3, 2023

Why does yark-pages need to do file stuff though, wouldn't any file stuff be the job of the API ?

When users want to load an archive that's not on the list of recent archives they need the file browser to popup so that they can select their archive. This needs tauri fs stuff because the webview returns C:\fake_path\x no matter the actual path if you try to do it nicely with normal html/js so to get around this tauri provides its own native api. There's no way to get around this limitation on browsers.

What could be potentially done is deploying two versions of the GUI. One using tauri and one with all of the archive loading stripped out, only allowing you to connect to a remote archive. The main problem with doing this is that I don't have time right now to figure out how to feature-gate sveltekit stuff depending on deployment targets and setup all of the deployment stuff and new designs for an online version.

@na4ma4
Copy link
Author

na4ma4 commented Feb 5, 2023

I'd see it as something you'd query a yark-api job, yark-pages would ask a yark-api "what archives do you have ?" and it would return a list.

So you could potentially point a yark-pages at multiple yark-api servers, or bundle them as one app and yark-pages still just asks yark-api to do the leg work.

@na4ma4 na4ma4 requested a review from Owez February 5, 2023 06:46
@Owez
Copy link
Owner

Owez commented Feb 5, 2023

I'd see it as something you'd query a yark-api job, yark-pages would ask a yark-api "what archives do you have ?" and it would return a list.

So you could potentially point a yark-pages at multiple yark-api servers, or bundle them as one app and yark-pages still just asks yark-api to do the leg work.

Yep exactly :)

image

I think of it in the way where you have CoD and if you want to play a game your client needs to connect to a server. Normally it connects to a local server it spins up itself from the server inside of it's own game files, but it can also connect to friend's servers.

I started to make a discovery system so you can find popular archives, but it became feature creep for this first version. There's also some considerations with people putting not very nice words in as their archive name. Hopefully I'll get around to adding it in the future.

@Owez
Copy link
Owner

Owez commented Feb 5, 2023

when this PR is ready I'll rebase it before you merge it.

I squash and merge into master fyi, not sure if a rebase is needed if I understand you:

image

@Owez Owez merged commit 1123b8b into Owez:master Feb 12, 2023
@na4ma4
Copy link
Author

na4ma4 commented Feb 13, 2023

I didn't get back to it in time, but it's all done now :)

I'll wait for 1.3.x to get more fleshed out before I start looking for things to help with.

@Owez
Copy link
Owner

Owez commented Feb 13, 2023

Nice, I think migrating docker support to the new yark-api thing would be for the best once everythings finished with it. Yesterday I made a lot of improvements to making development nicer using make (https://github.com/Owez/yark/blob/sveltekit/CONTRIBUTING.md#setting-up-development) so hopefully having make docker could build a docker container in the future, then the ci could use that?

@na4ma4
Copy link
Author

na4ma4 commented Feb 14, 2023

I've used the makefiles.dev makefiles before, make docker works pretty well with that (then make docker-push).

@Owez Owez mentioned this pull request Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants