feature: add git service token management #1136

studioego · 2023-03-07T07:24:06Z

feature: add git service token management

Initializing git_tokens table
make git_tokens API call (mockup)
CRUD feature with git_tokens table

Initializing git_tokens table

make git token manage API /func/git-tokens * GET : get git token list * PATCH: modify git tokens

fix select uuid on SQLAlchemy

…gement

Refactor get token list

modify to insert git token data using key-value json data

add git token management queries - delete non-exist domains - insert git service tokens (if it exists, update data)

…-management

…gement

when update git_token, fill modified_at date

Add news fragment

…gement

fix alembic's multiple heads

fregataa

I left some comment!

Plus, it will be good to make a new alembic migration file rather than merge them because we had lots of work to resolve migration sequence between 22.09 and 23.03. I am afraid that this merged file will cause problems.

fregataa · 2023-08-30T07:32:55Z

src/ai/backend/manager/models/git_token.py

+git_tokens = sa.Table(
+    "git_tokens",
+    metadata,
+    sa.Column(
+        "user_id",
+        GUID,
+        sa.ForeignKey("users.uuid", onupdate="CASCADE", ondelete="CASCADE"),
+        index=True,
+        nullable=False,
+        primary_key=True,
+    ),
+    sa.Column("domain", sa.String(length=200), unique=True, primary_key=True, nullable=False),
+    sa.Column("token", sa.String(length=200)),
+    sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now()),
+    sa.Column(
+        "modified_at",
+        sa.DateTime(timezone=True),
+        server_default=sa.func.now(),
+        onupdate=sa.func.current_timestamp(),
+    ),
+)


Since we decided to use ORM table, please define the table in ORM
This is an example code of our Session table.

fregataa · 2023-08-30T07:33:32Z

src/ai/backend/manager/models/git_token.py

+class GitTokenRow(Base):
+    __table__ = git_tokens


If we define the table in ORM, we can remove this overriding part.

fregataa · 2023-08-30T07:49:30Z

src/ai/backend/manager/api/userconfig.py

+            t.Key("params"): t.String,
+        },
+    )
+)
+async def update_git_tokens(request: web.Request, params: Any) -> web.Response:
+    resp = []
+    root_ctx: RootContext = request.app["_root.context"]
+    user_uuid = request["user"]["uuid"]
+
+    # get token list then
+    token_list = params["params"]


Suggested change

t.Key("params"): t.String,

},

)

)

async def update_git_tokens(request: web.Request, params: Any) -> web.Response:

resp = []

root_ctx: RootContext = request.app["_root.context"]

user_uuid = request["user"]["uuid"]

# get token list then

token_list = params["params"]

t.Key("tokens"): t.Mapping(t.String, t.String),

},

)

)

async def update_git_tokens(request: web.Request, params: Any) -> web.Response:

resp = []

root_ctx: RootContext = request.app["_root.context"]

user_uuid = request["user"]["uuid"]

# get token dictionary then

tokens: dict[str, str] = params["tokens"]

First, the name of parameter is too vague. Let's change it.
If we successfully change the parameter type, then let's update git_token.insert_update_git_tokens()!

fregataa · 2023-08-30T07:52:41Z

src/ai/backend/manager/api/userconfig.py

+    user_uuid = request["user"]["uuid"]
+
+    async with root_ctx.db.begin_readonly() as conn:
+        query = sa.select([git_tokens.c.domain, git_tokens.c.token]).where(
+            git_tokens.c.user_id == user_uuid
+        )


Is there any use case of getting other user's git tokens?
For example, admins would like to get other user's tokens.

fregataa · 2023-08-30T07:53:15Z

src/ai/backend/manager/api/userconfig.py

+async def update_git_tokens(request: web.Request, params: Any) -> web.Response:
+    resp = []
+    root_ctx: RootContext = request.app["_root.context"]
+    user_uuid = request["user"]["uuid"]


Same here.
Please let me know if there is any use case of getting or updating other user's git token,

fregataa · 2023-08-30T07:58:16Z

src/ai/backend/manager/models/git_token.py

+        primary_key=True,
+    ),
+    sa.Column("domain", sa.String(length=200), unique=True, primary_key=True, nullable=False),
+    sa.Column("token", sa.String(length=200)),


Just a question!
How long git token usually is??

fregataa · 2023-08-30T08:00:44Z

src/ai/backend/manager/models/git_token.py

+    sa.Column(
+        "modified_at",
+        sa.DateTime(timezone=True),
+        server_default=sa.func.now(),


I think that this field should be nullable and default value should be null if we think of a token row without any modification

fregataa · 2023-08-30T08:01:31Z

src/ai/backend/manager/models/git_token.py

+            "user_id": user_uuid,
+            "domain": key,
+            "token": value,
+            "modified_at": datetime.datetime.now(),


I think that we do not need to fill this part because of the field's onupdate function.

fregataa · 2023-08-30T08:03:53Z

src/ai/backend/manager/models/git_token.py

+    token_list = json.loads(token_list_str)
+    domain_list = []


This variable is actually of type dict, but it has list in its name, which confuses me into thinking it is of type list.
let's rename them and add type hint!

Suggested change

token_list = json.loads(token_list_str)

domain_list = []

tokens: dict[str, Any] = json.loads(token_list_str)

domain_list: list[str] = []

fregataa · 2023-08-30T08:09:31Z

src/ai/backend/manager/models/git_token.py

+    sa.Column(
+        "user_id",
+        GUID,
+        sa.ForeignKey("users.uuid", onupdate="CASCADE", ondelete="CASCADE"),
+        index=True,
+        nullable=False,
+        primary_key=True,
+    ),


Is git token unique for every user?

fregataa · 2023-08-30T08:39:40Z

src/ai/backend/manager/models/git_token.py

+    token_list = json.loads(token_list_str)
+    domain_list = []
+    # check domain_list
+    for key, value in token_list.items():


Suggested change

for key, value in token_list.items():

for domain_name, token in token_list.items():

Let's use more clear name!

fregataa · 2023-08-30T08:39:55Z

src/ai/backend/manager/models/git_token.py

+    )
+    await conn.execute(delete_stmt)
+
+    for key, value in token_list.items():


Suggested change

for key, value in token_list.items():

for domain_name, token in token_list.items():

Same here

fregataa · 2023-08-30T08:41:11Z

src/ai/backend/manager/models/git_token.py

+        nullable=False,
+        primary_key=True,
+    ),
+    sa.Column("domain", sa.String(length=200), unique=True, primary_key=True, nullable=False),


And domains table has name field which is a primary key.
How about defining this field as a ForeignKey?

studioego · 2023-08-30T16:22:27Z

I left some comment!

Plus, it will be good to make a new alembic migration file rather than merge them because we had lots of work to resolve migration sequence between 22.09 and 23.03. I am afraid that this merged file will cause problems.

According to your opinion, I make a new alembic version.
Because, merged file can easily cause problems.

make a new alembic migration file because, merge alembic working can easily broke migration sequence

…gement

update alembic heads

fix the broken alembic heads

Initializing git_tokens table

daf7d1d

Initializing git_tokens table

studioego added type:refactor Refactoring current implementation. type:enhance Enhance component, behavior, internals without user-facing features labels Mar 7, 2023

studioego self-assigned this Mar 7, 2023

github-actions bot added the comp:manager Related to Manager component label Mar 7, 2023

make git token manage API

8620616

make git token manage API /func/git-tokens * GET : get git token list * PATCH: modify git tokens

studioego force-pushed the feature/add-git-token-management branch from 1183d16 to 8620616 Compare March 13, 2023 17:48

studioego added 7 commits March 16, 2023 00:07

fix select uuid on SQLAlchemy

06bfccd

fix select uuid on SQLAlchemy

Merge remote-tracking branch 'origin' into feature/add-git-token-mana…

8437067

…gement

Refactor get token list

9aa48cd

Refactor get token list

modify to insert git token data

112e0a5

modify to insert git token data using key-value json data

add git token management queries

7cc63f6

add git token management queries - delete non-exist domains - insert git service tokens (if it exists, update data)

Merge remote-tracking branch 'origin/main' into feature/add-git-token…

b756272

…-management

Merge remote-tracking branch 'origin/main' into feature/add-git-token…

49e6237

…-management

Yaminyam added the size:L 100~500 LoC label Apr 13, 2023

studioego added 3 commits June 27, 2023 19:58

Merge remote-tracking branch 'origin' into feature/add-git-token-mana…

a5cde53

…gement

Merge remote-tracking branch 'origin' into feature/add-git-token-mana…

0bc7571

…gement

when update git_token, fill modified_at date

46634af

when update git_token, fill modified_at date

github-actions bot added the require:db-migration Automatically set when alembic migrations are added or updated label Aug 29, 2023

studioego marked this pull request as ready for review August 29, 2023 08:06

studioego requested review from fregataa and achimnol August 29, 2023 08:06

studioego added 2 commits August 30, 2023 14:32

Add news fragment

8b470a0

Add news fragment

Merge remote-tracking branch 'origin' into feature/add-git-token-mana…

a44a495

…gement

studioego force-pushed the feature/add-git-token-management branch from a1f1637 to a44a495 Compare August 30, 2023 05:47

fix alembic's multiple heads

f40a623

fix alembic's multiple heads

studioego mentioned this pull request Aug 30, 2023

feature: add git service token management & importing git service refectoring lablup/backend.ai-webui#1620

Draft

7 tasks

studioego requested a review from adrysn August 30, 2023 07:12

fregataa requested changes Aug 30, 2023

View reviewed changes

fregataa reviewed Aug 30, 2023

View reviewed changes

make a new alembic migration file

a80ede2

make a new alembic migration file because, merge alembic working can easily broke migration sequence

studioego force-pushed the feature/add-git-token-management branch from ad30e01 to a80ede2 Compare August 30, 2023 16:25

studioego and others added 6 commits August 31, 2023 01:31

Merge remote-tracking branch 'origin' into feature/add-git-token-mana…

51ac4c4

…gement

update alembic heads

b6018ac

update alembic heads

fix the broken alembic heads

fb98861

fix the broken alembic heads

Merge branch 'main' into feature/add-git-token-management

d7e52c2

Merge branch 'main' into feature/add-git-token-management

f3e4c84

Merge branch 'main' into feature/add-git-token-management

0420a63

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add git service token management #1136

feature: add git service token management #1136

studioego commented Mar 7, 2023 •

edited

fregataa left a comment

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023 •

edited

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

fregataa Aug 30, 2023

studioego commented Aug 30, 2023

	for key, value in token_list.items():
	for domain_name, token in token_list.items():

feature: add git service token management #1136

Are you sure you want to change the base?

feature: add git service token management #1136

Conversation

studioego commented Mar 7, 2023 • edited

fregataa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fregataa Aug 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

studioego commented Aug 30, 2023

studioego commented Mar 7, 2023 •

edited

fregataa Aug 30, 2023 •

edited