Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

feat: Update CRUD of session template. #480

Merged
merged 33 commits into from
Nov 22, 2021
Merged

Conversation

agatha197
Copy link
Contributor

  • Modified to create/update multiple session templates at once
  • Fields such as template or domain_name can also be read or updated.
  • Correct typo of example-session-templates.json

Related: control-panel#120, webui#1133

@codecov
Copy link

codecov bot commented Oct 1, 2021

Codecov Report

Merging #480 (14ac2bd) into main (15acec7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #480   +/-   ##
=======================================
  Coverage   48.89%   48.89%           
=======================================
  Files          54       54           
  Lines        8908     8908           
=======================================
  Hits         4356     4356           
  Misses       4552     4552           
Impacted Files Coverage Δ
src/ai/backend/manager/models/session_template.py 31.64% <ø> (ø)

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 15acec7...14ac2bd. Read the comment docs.

@agatha197 agatha197 changed the title feat: update session template crud feat: Update CRUD of session template. Oct 1, 2021
fix: delete debugging codes.

fix: lint errors
@agatha197 agatha197 force-pushed the feature/CRUD-session-template branch from d0dd25f to e748010 Compare October 15, 2021 09:43
@achimnol achimnol added this to the 21.09 milestone Oct 16, 2021
Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, but please append trailing commas. 😉

src/ai/backend/manager/api/session_template.py Outdated Show resolved Hide resolved
src/ai/backend/manager/api/session_template.py Outdated Show resolved Hide resolved
src/ai/backend/manager/api/session_template.py Outdated Show resolved Hide resolved
@agatha197 agatha197 force-pushed the feature/CRUD-session-template branch from 5338711 to 9a6b4ee Compare November 10, 2021 02:44
@agatha197 agatha197 requested review from adrysn and achimnol November 10, 2021 03:11
Copy link
Member

@adrysn adrysn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and it works. LGTM for me.

Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check whether checking the emptiness of query results is actually executed or not.
I think it should be rewritten to count the number of returned rows instead of performing None-check inside the for-loop against the result.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants