saltyorg / Sandbox

Saltbox Sandbox
GNU General Public License v3.0
66 stars 92 forks source link

Add Codex #237

Closed RaneyDazed closed 10 months ago

RaneyDazed commented 1 year ago

Description

Adding Codex, as recently requested. It adds the volume, /comics to /mnt/unionfs/Media/Comics as currently written.

How Has This Been Tested?

Installed via docker compose initially, then installed via saltbox_mod. That is how I'm currently running it. Works fine and I've not gotten any errors so far. the user mapping seems to be working fine, and its still scanning my comic directory.

RaneyDazed commented 1 year ago

don't approve yet, not sure if I'm having trouble due to not setting up my user in the UI or if its something else. :p works fine via compose, need to dick w the role for a few

RaneyDazed commented 1 year ago

Ok, it works as expected now. Pulls up paths and allows you to scan your comics directory.

RaneyDazed commented 1 year ago

236

Mariachi2100 commented 1 year ago

Awesome. Thank you for your efforts. Much appreciated.

RaneyDazed commented 1 year ago

Awesome. Thank you for your efforts. Much appreciated.

Happy to help! Just gotta wait for someone to review it and make sure its ok with the maintainers and it will go live. : )

maximuskowalski commented 1 year ago

Did you check if it HAS to be /comics for the media dir or can you use whatever you want?

RaneyDazed commented 1 year ago

I'll take another quick look. just a sec

RaneyDazed commented 1 year ago

I didn't look too deep into it as I'm in bed rn, but this is the compose from their "docker" setup on docker hub:

services:
  codex:
      env_file: .env
      image: docker.io/ajslater/codex
      container_name: codex
      volumes:
          - <host path to data>:/config
          - <host path to comics>:/comics:ro
      ports:
          - 9810:9810
      restart: on-failure
RaneyDazed commented 1 year ago
Screenshot 2023-03-22 at 11 23 44 PM
RaneyDazed commented 1 year ago

Might be too late and my brain isn't processing the question correctly. when you ask if it needs to be mapped to /comics you mean to ask, do they allow the user to make /comics anything they want? because if so, I have no idea. I haven't seen anything that says we can. But I haven't seen anything that says we can't either. I can try to make it whatever tomorrow at work when I should be working : p

maximuskowalski commented 1 year ago

Yeh, so I think I would add /mnt:/mnt to this so people can also use whatever library paths they want or might already have. Also need to think about how we add this to the traefik branch at the same time, so don't delete the branch after it gets merged, put in another PR for the traefik3 branch and we can edit it to work there as well.

We should check we are still adding mnt:/mnt and not /mnt/Media for this sort of situation too. There is of course the custom inventory option too.

RaneyDazed commented 1 year ago

Yeh, so I think I would add /mnt:/mnt to this so people can also use whatever library paths they want or might already have. Also need to think about how we add this to the traefik branch at the same time, so don't delete the branch after it gets merged, put in another PR for the traefik3 branch and we can edit it to work there as well.

We should check we are still adding mnt:/mnt and not /mnt/Media for this sort of situation too. There is of course the custom inventory option too.

I can change that up, no prob. Should I go around and change the other sandbox roles w the full path on them? Maybe not, since it could screw up how it’s currently configured?

RaneyDazed commented 1 year ago

I was scratching my head looking for the default docker volume with mnt and couldn't find it. that pesky max came by and did it for me :p tyvm max!

maximuskowalski commented 1 year ago

I was actually under the impression that /mnt:/mnt had been removed and was frowned upon being added to all roles so I too was scratching.

saltydk commented 1 year ago

We just added an opt-out variable from what I remember

On Tue, 27 Jun 2023, 00:52 Max Kowalski, @.***> wrote:

I was actually under the impression that /mnt:/mnt had been removed and was frowned upon being added to all roles so I too was scratching.

— Reply to this email directly, view it on GitHub https://github.com/saltyorg/Sandbox/pull/237#issuecomment-1608424085, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSIMLVRUPLM4VA4YDOJ5FTXNIHBTANCNFSM6AAAAAAWECZNXE . You are receiving this because you commented.Message ID: @.***>

RaneyDazed commented 1 year ago

Yeh, so I think I would add /mnt:/mnt to this so people can also use whatever library paths they want or might already have. Also need to think about how we add this to the traefik branch at the same time, so don't delete the branch after it gets merged, put in another PR for the traefik3 branch and we can edit it to work there as well.

We should check we are still adding mnt:/mnt and not /mnt/Media for this sort of situation too. There is of course the custom inventory option too.

I think I did this without thinking :p

maximuskowalski commented 11 months ago

Are we all good with this one now RD? And we have a docs PR too?

RaneyDazed commented 11 months ago

hmm I'll have to see if I've made that already or not. I'll take a look. It shouldn't be too difficult to get done. : ) but yes all set on this one! Aside from the docs PR

Are we all good with this one now RD? And we have a docs PR too?

saltydk commented 10 months ago

@maximuskowalski @owine review this before the end of the weekend, merge it to traefik3 or close it.

owine commented 10 months ago

This was merged without a direct test - any issues with the role, please ping me in Discord to troubleshoot