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 image #129

Open
dantownsend opened this issue Nov 17, 2021 · 13 comments
Open

Docker image #129

dantownsend opened this issue Nov 17, 2021 · 13 comments
Labels
enhancement New feature or request help wanted Extra attention is needed proposal - input needed An idea for a new feature which requires input

Comments

@dantownsend
Copy link
Member

dantownsend commented Nov 17, 2021

It would be really cool if we had a Docker image for Piccolo Admin.

It would have environment variables such as: PG_HOST, PG_PASSWORD etc to tell it how to connect to Postgres.

To determine which tables to connect to, it would either use a config file mounted in a volume, or would accept another environment variable called PICCOLO_TABLES. For example PICCOLO_TABLES=article,post.

Since Piccolo now has powerful schema reflection abilities, we can generate the Piccolo Table classes automatically.

It would open up Piccolo admin to a much wider user base (including non-Python users).

@dantownsend dantownsend added enhancement New feature or request proposal - input needed An idea for a new feature which requires input labels Nov 17, 2021
@dantownsend dantownsend added the help wanted Extra attention is needed label Nov 17, 2021
@sinisaos
Copy link
Member

@dantownsend This is a great idea because Piccolo Admin can be the admin UI for any existing Postgres database (of course the database must have columns that are supported by Piccolo ORM). I don't know anything about Docker so there won't be any help from me, but I like the idea.

@dantownsend
Copy link
Member Author

@sinisaos Yeah, I'm excited about the possibilities.

@Skelmis
Copy link
Contributor

Skelmis commented Aug 26, 2024

Working on this, some issues as I go:

auto_include_related=True generates extra, duplicate table configs

Unsure why, haven't looked into it much

Array types don't appear to infer well. Given:
    code_scanners = Array(
        base_column=Text(),
        help_text="A list of Analysis Interface id's to use",
    )

Such that a column contains:
image

Then the following is raised:

  File "/home/skelmis/Code/Python/piccolo_admin_dockered/venv/lib/python3.11/site-packages/piccolo_api/crud/endpoints.py", line 926, in get_all
    json = self.pydantic_model_plural(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/skelmis/Code/Python/piccolo_admin_dockered/venv/lib/python3.11/site-packages/pydantic/main.py", line 193, in __init__
    self.__pydantic_validator__.validate_python(data, self_instance=self)
pydantic_core._pydantic_core.ValidationError: 1 validation error for ProjectPlural
rows.0.code_scanners
  Input should be a valid integer [type=int_type, input_value=['bandit', 'semgrep'], input_type=list]
    For further information visit https://errors.pydantic.dev/2.8/v/int_type
Accessing the admin interface without creds

It's not always possible creds exist, and we dont want to create a piccolo user so bootstrapping the UI to work so needing to add another flag to whether or not to put behind auth before modifying /api and get_user. I may move more towards an argument for custom auth provider instead of an on/off configuration I feel

Giving TableStorage an Engine

get_piccolo_conf_module fails because well, we don't have one setup. So needed to patch in the ability to directly provide the engine to TableStorage


Other then that I've got a fairly decent standalone thing working. Next steps are to dockerize it and provide table input via env etc

Code link specifically here

But yea, it's pre cool. Works relatively okay at face value

@dl-lim
Copy link

dl-lim commented Oct 18, 2024

Hi there, I've been looking for something similar to this project, a UI for a database, and would like to deploy it on docker. Thought I'd contribute what I can.

To start with, a simple dockerised admin_ui: #427

Appreciate some guidance on merging @Skelmis's standalone to this.

@Skelmis
Copy link
Contributor

Skelmis commented Oct 18, 2024

Happy to take a look, my ideas weren't exactly well refined haha. Remind me in a few days if I haven't had a look yet and I can aim to give some feedback

@dantownsend
Copy link
Member Author

@Skelmis Sorry, I totally missed your original comment. It sounds like you made good progress.

I'm aware that table reflection doesn't work great at the moment for array columns - it's something I need to look into.

As for authentication, we could do something like:

  1. Make the user specify a Postgres schema within their database where we can create the Piccolo user and session auth tables (e.g. instead of public, it could be piccolo). That way the Piccolo Admin tables are separate to their main database.
  2. We create the Piccolo user and session auth tables in a separate SQLite database.

It might be that we provide several options to the user. Option 2 might actually be easiest as a default option. create_admin has auth_table and session_table arguments, and we can pass in subclasses of BaseUser and SessionsBase which use the local SQLite database.

from piccolo.apps.user.tables import BaseUser
from piccolo.engine.sqlite import SQLiteEngine
from piccolo_api.session_auth.tables import SessionsBase
from piccolo_admin.endpoints import create_admin

DB = SQLiteEngine('piccolo_admin_auth.sqlite')

class User(BaseUser, db=DB):
    ...

class Sessions(SessionsBase, db=DB):
    ...

admin = create_admin(auth_table=User, session_table=Sessions)

get_piccolo_conf_module fails because well, we don't have one setup. So needed to patch in the ability to directly provide the engine to TableStorage

Ah, good point - I think that's an easy fix, will create a ticket in Piccolo to add that!

@dantownsend
Copy link
Member Author

@dl-lim Thanks for creating the PR and your interest in this. I think it is a cool feature.

@dantownsend
Copy link
Member Author

@Skelmis Here's a PR adding an engine option to TableStorage:

piccolo-orm/piccolo#1109

@sinisaos
Copy link
Member

It might be that we provide several options to the user. Option 2 might actually be easiest as a default option. create_admin has auth_table and session_table arguments, and we can pass in subclasses of BaseUser and SessionsBase which use the local SQLite database.

@dantownsend That works. I played around with this a bit. I created an admin user with a separate Sqlite database and after that Piccolo auth works. I used the code from @Skelmis PR code. Here is the github repo. Maybe will be useful.

@dantownsend
Copy link
Member Author

@sinisaos That's amazing, thanks!

Do you think it's best to have a dedicated repo for it, or add it to the main piccolo_admin repo?

Some observations from your repo, which we can add next:

  • Add ENV values do Dockerfile, to reflect the environment variables we're reading in
  • Add VOLUME wherever the SQLite database is stored
  • Have some way of the user specifying which tables to reflect

As Piccolo Admin has so many options, we need some way of the user providing these in the future. We could have a bunch of environment variables, but should also consider having a YAML or JSON file which the user can add to a volume. Ideally we would have a simple UI which guides the user through the configuration, and then outputs the JSON file, or writes it to the SQLite database.

@sinisaos
Copy link
Member

@dantownsend Thank you very much.

Do you think it's best to have a dedicated repo for it, or add it to the main piccolo_admin repo?

I think a dedicated repo will be a better solution, just like Piccolo Docker

Some observations from your repo, which we can add next:

  • Add ENV values do Dockerfile, to reflect the environment variables we're reading in
  • Add VOLUME wherever the SQLite database is stored
  • Have some way of the user specifying which tables to reflect

As Piccolo Admin has so many options, we need some way of the user providing these in the future. We could have a bunch of environment variables, but should also consider having a YAML or JSON file which the user can add to a volume. Ideally we would have a simple UI which guides the user through the configuration, and then outputs the JSON file, or writes it to the SQLite database.

I agree with you, but I don't have much experience with Docker. In the repo I posted I just want to show that your idea with adding the admin user to a separate Sqlite db, works (after that we can use the Piccolo Admin session auth).

@dantownsend
Copy link
Member Author

@sinisaos OK, lets put in a separate repo for now then.

Do you want to transfer your repo to the Piccolo org, or should we start a new one?

@sinisaos
Copy link
Member

sinisaos commented Oct 24, 2024

@dantownsend I've done the transfer and the new repo is here. I hope that's okay. If not, just rename it, remove it, or do whatever you think is best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed proposal - input needed An idea for a new feature which requires input
Projects
None yet
Development

No branches or pull requests

4 participants