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

Add Codex #237

Merged
merged 5 commits into from
Sep 16, 2023
Merged

Add Codex #237

merged 5 commits into from
Sep 16, 2023

Conversation

RaneyDazed
Copy link
Contributor

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.

  • Tested on my feeder, and tested on my aio/mediabox.

@RaneyDazed
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@RaneyDazed
Copy link
Contributor Author

#236

@Mariachi2100
Copy link

Awesome. Thank you for your efforts. Much appreciated.

@RaneyDazed
Copy link
Contributor Author

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
Copy link
Collaborator

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

@RaneyDazed
Copy link
Contributor Author

I'll take another quick look. just a sec

@RaneyDazed
Copy link
Contributor Author

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
Copy link
Contributor Author

Screenshot 2023-03-22 at 11 23 44 PM

@RaneyDazed
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

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 and others added 2 commits March 28, 2023 08:25
Remove full “Media” path from `paths`. Changed full “Media” path mapping (docker volumes) to /mnt. Removed /comics volume.
trigger ci
remove /mnt
@RaneyDazed
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Member

saltydk commented Jun 27, 2023 via email

@RaneyDazed
Copy link
Contributor Author

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
Copy link
Collaborator

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

@RaneyDazed
Copy link
Contributor Author

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?

@maximuskowalski maximuskowalski added the documentation needed waiting on documentation PR label Aug 12, 2023
@saltydk
Copy link
Member

saltydk commented Sep 16, 2023

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

@owine owine merged commit 3f8c5a6 into saltyorg:master Sep 16, 2023
owine added a commit that referenced this pull request Sep 16, 2023
* Add Codex, def and tasks, add to sandbox.yml

* Corrected envs PUID & PGID

* Update main.yml

Remove full “Media” path from `paths`. Changed full “Media” path mapping (docker volumes) to /mnt. Removed /comics volume.

* Update main.yml

trigger ci

* Update main.yml

remove /mnt

---------

Co-authored-by: Max Kowalski <13492750+maximuskowalski@users.noreply.github.com>
@owine
Copy link
Collaborator

owine commented Sep 16, 2023

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

@RaneyDazed RaneyDazed deleted the Add-codex branch November 3, 2023 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation needed waiting on documentation PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants