feat: add start_service API #479

kyujin-cho · 2021-09-16T09:29:10Z

this commit adds new start_service API under session, which starts kernel service on backend.ai session and returns service access information (host and port).

codecov · 2021-09-16T09:29:16Z

Codecov Report

Merging #479 (8890a43) into main (d8d525a) will increase coverage by 0.01%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
+ Coverage   48.84%   48.85%   +0.01%     
==========================================
  Files          54       54              
  Lines        8943     8947       +4     
==========================================
+ Hits         4368     4371       +3     
- Misses       4575     4576       +1

Impacted Files	Coverage Δ
src/ai/backend/manager/models/scaling_group.py	`65.82% <75.00%> (+0.15%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8d525a...8890a43. Read the comment docs.

… user

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

adrysn · 2021-10-11T02:34:35Z

...i/backend/manager/models/alembic/versions/60a1effa77d2_add_coordinator_address_column_on_.py

+
+
+def upgrade():
+    op.add_column('scaling_groups', sa.Column('wsproxy_address', sa.String(length=1024), nullable=True))


This is just a question. Is there a reason to set wsproxy_address per scaling group? I originally thought that the wsproxy endpoint can be configured globally from manager.toml or somewhere else. So, I wonder if there a scenario for multiple wsproxies for a different scaling group.

For more scalability and distributed deployments, I think it would be better to have this option, although in most cases it would fallback to the manager.toml configuration value.
Note: In the future this would be refactored using RBAC (also the default storage proxy for scaling groups / allowed storage proxies for user groups as well).

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

achimnol

Just a minor change please in the code level.

achimnol

Sorry again, but another minor fix suggestion: rename wsproxy_address to wsproxy_addr, to be consistent with agent_addr in other places.

adrysn

Found a minor typo.

...i/backend/manager/models/alembic/versions/60a1effa77d2_add_coordinator_address_column_on_.py

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

achimnol

Please make the Redis-based caching logic to "stand out", i.e., any new code readers could easily recognize it as a caching logic.

achimnol

Just a minor refactoring please.
Other parts looks good!

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

This API is dedicated to start a container application from container's service definitions, while existing `stream_proxy` API performs both `start_service` and then actual websocket proxying, for wsproxy v2. Note: wsproxy v2 directly connects the user clients and the container apps to provide better horizontal scalability. The separation of those two functions is the key starting point. Co-authored-by: Jonghyun Park <[email protected]> Backported-From: main (22.03) Backported-To: 21.09

Add new start_service API

cc38070

kyujin-cho requested a review from achimnol September 16, 2021 09:29

kyujin-cho added 3 commits September 16, 2021 18:29

Fix flake8 error

4080ded

Add towncrier changelog

2995ff4

Call wsproxy coordinator directly rather than exposing kernel info to…

3a7d42b

… user

kyujin-cho mentioned this pull request Sep 19, 2021

Add support for v2 (direct connection) wsproxy mode lablup/backend.ai-webui#1130

Merged

kyujin-cho added 2 commits September 19, 2021 18:10

Fix linting error

eb9d29d

Rename confusing keywords

5636605

adrysn assigned kyujin-cho Oct 11, 2021

adrysn added this to the 21.09 milestone Oct 11, 2021

adrysn reviewed Oct 11, 2021

View reviewed changes

src/ai/backend/manager/api/session.py Outdated Show resolved Hide resolved

adrysn reviewed Oct 11, 2021

View reviewed changes

achimnol reviewed Oct 11, 2021

View reviewed changes

src/ai/backend/manager/api/session.py Outdated Show resolved Hide resolved

achimnol suggested changes Oct 11, 2021

View reviewed changes

Accept PR Reviews

7cbb87f

achimnol suggested changes Oct 12, 2021

View reviewed changes

kyujin-cho added 4 commits October 13, 2021 12:29

Merge branch 'main' into feature/direct-wsproxy-connection

a08a250

Add wsproxy-version API which returns target wsproxy version string

c5b7e94

Fix invalid Redis API usage

36bb3e6

Rename wsproxy_address to wsproxy_addr

880ed52

adrysn suggested changes Oct 15, 2021

View reviewed changes

...i/backend/manager/models/alembic/versions/60a1effa77d2_add_coordinator_address_column_on_.py Outdated Show resolved Hide resolved

adrysn added 2 commits October 17, 2021 10:39

fix: typo in alembic downgrade function for wsproxy_addr

220814b

fix: error when scaling_groups.c.wsproxy_addr is am empty string ("")

0693aa4

adrysn added API db-migration-required feature labels Oct 17, 2021

adrysn and others added 3 commits October 17, 2021 17:45

Merge branch 'main' into feature/direct-wsproxy-connection

5ba6d96

fix: flake8 errors (trailing comma)

2ae9e01

docs: Update news fragment

7c11987

achimnol approved these changes Oct 18, 2021

View reviewed changes

achimnol reviewed Oct 18, 2021

View reviewed changes

src/ai/backend/manager/api/scaling_group.py Outdated Show resolved Hide resolved

achimnol suggested changes Oct 18, 2021

View reviewed changes

kyujin-cho added 2 commits October 18, 2021 17:52

Use aiotools' cache helper

9e398de

Set cache expire time to 30 seconds

0834444

achimnol added feature and removed feature labels Nov 25, 2021

kyujin-cho added 2 commits December 13, 2021 17:52

Merge branch 'main' into feature/direct-wsproxy-connection

9ae4f07

Merge branch 'main' into feature/direct-wsproxy-connection

505e19a

achimnol suggested changes Dec 17, 2021

View reviewed changes

src/ai/backend/manager/api/scaling_group.py Outdated Show resolved Hide resolved

Use dataclass instead of DummyHashObject

8890a43

achimnol approved these changes Dec 17, 2021

View reviewed changes

achimnol merged commit 06b69a0 into main Dec 17, 2021

achimnol deleted the feature/direct-wsproxy-connection branch December 17, 2021 02:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add start_service API #479

feat: add start_service API #479

kyujin-cho commented Sep 16, 2021

codecov bot commented Sep 16, 2021 •

edited

Loading

adrysn Oct 11, 2021 •

edited

Loading

achimnol Oct 11, 2021

achimnol left a comment

achimnol left a comment •

edited

Loading

adrysn left a comment

achimnol left a comment •

edited

Loading

achimnol left a comment



		def upgrade():
		op.add_column('scaling_groups', sa.Column('wsproxy_address', sa.String(length=1024), nullable=True))

feat: add start_service API #479

feat: add start_service API #479

Conversation

kyujin-cho commented Sep 16, 2021

codecov bot commented Sep 16, 2021 • edited Loading

Codecov Report

adrysn Oct 11, 2021 • edited Loading

Choose a reason for hiding this comment

achimnol Oct 11, 2021

Choose a reason for hiding this comment

achimnol left a comment

Choose a reason for hiding this comment

achimnol left a comment • edited Loading

Choose a reason for hiding this comment

adrysn left a comment

Choose a reason for hiding this comment

achimnol left a comment • edited Loading

Choose a reason for hiding this comment

achimnol left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 16, 2021 •

edited

Loading

adrysn Oct 11, 2021 •

edited

Loading

achimnol left a comment •

edited

Loading

achimnol left a comment •

edited

Loading