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

fix: slot stats are not filled in everywhere #9070

Merged
merged 3 commits into from
Mar 28, 2024
Merged

Conversation

hamidzr
Copy link
Contributor

@hamidzr hamidzr commented Mar 28, 2024

Description

I added a summary of slots to the agent responses and annotated it as always available resulting the generated client code to expect that in some other apis I did not update.

issue was caught before being released

Test Plan

ee run with this cherry-picked https://app.circleci.com/pipelines/github/determined-ai/determined-ee/11396/workflows/aff838c6-3eff-49cc-9d2f-8735bbd04ee2/jobs/571167

call various endpoints getting the Agent object

proto/src/determined/api/v1/api.proto
284:  rpc GetAgents(GetAgentsRequest) returns (GetAgentsResponse) {
293:  rpc GetAgent(GetAgentRequest) returns (GetAgentResponse) {
320:  rpc EnableAgent(EnableAgentRequest) returns (EnableAgentResponse) {
329:  rpc DisableAgent(DisableAgentRequest) returns (DisableAgentResponse) {
hbp ➜ d/determined git:(main) det dev b call get_GetAgent agent1 > /dev/null
Usage: get_GetAgent <= agentId: str

'NoneType' object is not subscriptable
hbp ➜ d/determined git:(main) gco -
Switched to branch 'hz-slot-stats-can-be-null'
Your branch is up to date with 'origin/hz-slot-stats-can-be-null'.
hbp ➜ d/determined git:(hz-slot-stats-can-be-null) det dev b call get_GetAgent agent1 > /dev/null
hbp ➜ d/determined git:(hz-slot-stats-can-be-null)

Commentary (optional)

https://hpe-aiatscale.slack.com/archives/C04C9JXB1C2/p1711646351359479

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

@cla-bot cla-bot bot added the cla-signed label Mar 28, 2024
@hamidzr hamidzr added to-cherry-pick Pull requests that need to be cherry-picked into the current release and removed cla-signed labels Mar 28, 2024
Copy link

netlify bot commented Mar 28, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 78bac61
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/6605c1f6abe0bc0008b9d229

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 31 lines in your changes are missing coverage. Please review.

Project coverage is 47.13%. Comparing base (1e45918) to head (78bac61).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9070      +/-   ##
==========================================
- Coverage   47.15%   47.13%   -0.03%     
==========================================
  Files        1152     1152              
  Lines      141866   141866              
  Branches     2416     2414       -2     
==========================================
- Hits        66899    66867      -32     
- Misses      74777    74809      +32     
  Partials      190      190              
Flag Coverage Δ
backend 42.92% <0.00%> (-0.08%) ⬇️
harness 64.31% <ø> (ø)
web 38.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
master/internal/api_agents.go 0.00% <ø> (-21.59%) ⬇️
master/pkg/model/agent.go 0.00% <0.00%> (ø)

... and 4 files with indirect coverage changes

@hamidzr hamidzr marked this pull request as ready for review March 28, 2024 19:29
@hamidzr hamidzr requested a review from a team as a code owner March 28, 2024 19:29
@hamidzr hamidzr requested a review from eecsliu March 28, 2024 19:29
Copy link
Contributor

@stoksc stoksc left a comment

Choose a reason for hiding this comment

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

lgtm

@hamidzr hamidzr merged commit 6c4bc44 into main Mar 28, 2024
80 of 94 checks passed
@hamidzr hamidzr deleted the hz-slot-stats-can-be-null branch March 28, 2024 20:07
dai-release bot pushed a commit that referenced this pull request Mar 28, 2024
causes unmarshalling of get agent and enable/disable agent to fail
in python generated client.
introduced in e8dba6d

(cherry picked from commit 6c4bc44)
@mackrorysd mackrorysd self-assigned this Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
to-cherry-pick Pull requests that need to be cherry-picked into the current release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants