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.

Sign up for GitHub

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

Jump to bottom

Add commands to list and retrieve additional components #3114

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Add commands to list and retrieve additional components #3114

wants to merge 13 commits into from
+165 −0

Conversation

nvkevlu
Copy link
Collaborator

@nvkevlu nvkevlu commented Dec 17, 2024

Add commands to list and retrieve additional components.

Description

After adding additional components to store the error logs (and additional components if implemented in the future), in order to list and download the components, additional admin commands are added.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Quick tests passed locally by running ./runtest.sh.
  • In-line docstrings updated.
  • Documentation updated.

@nvkevlu nvkevlu enabled auto-merge (squash) January 7, 2025 15:13
@nvkevlu
Copy link
Collaborator Author

nvkevlu commented Jan 13, 2025

/build

YuanTingHsieh
YuanTingHsieh reviewed Jan 13, 2025
View reviewed changes
Copy link
Collaborator

@YuanTingHsieh YuanTingHsieh left a comment

Choose a reason for hiding this comment

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

Have some questions regarding user-facing concepts/methods

nvflare/apis/impl/job_def_manager.py Outdated Show resolved Hide resolved
nvflare/apis/job_def_manager_spec.py Outdated Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Outdated Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Outdated Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Outdated Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Outdated Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Outdated Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Show resolved Hide resolved
yanchengnv
yanchengnv requested changes Jan 14, 2025
View reviewed changes
Copy link
Collaborator

@yanchengnv yanchengnv left a comment

Choose a reason for hiding this comment

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

Looks good in general. See my comments for improvement.

nvflare/apis/job_def_manager_spec.py Outdated Show resolved Hide resolved
nvflare/apis/impl/job_def_manager.py Outdated Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Show resolved Hide resolved
nvflare/private/fed/server/job_cmds.py Outdated Show resolved Hide resolved
@nvkevlu nvkevlu requested a review from yanchengnv January 14, 2025 21:18
yanchengnv
yanchengnv requested changes Jan 14, 2025
View reviewed changes
Copy link
Collaborator

@yanchengnv yanchengnv left a comment

Choose a reason for hiding this comment

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

See my comments to make the meta data easier for programmers to use.

nvflare/private/fed/server/job_cmds.py Outdated Show resolved Hide resolved
@nvkevlu nvkevlu requested a review from yanchengnv January 15, 2025 12:54
@nvkevlu
Copy link
Collaborator Author

nvkevlu commented Jan 15, 2025

/build

yanchengnv
yanchengnv requested changes Jan 15, 2025
View reviewed changes
Copy link
Collaborator

@yanchengnv yanchengnv left a comment

Choose a reason for hiding this comment

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

Must keep "visible=False" for download_job_file since this is an internal command and not to be exposed to users.

nvflare/private/fed/server/job_cmds.py Show resolved Hide resolved
@nvkevlu
Copy link
Collaborator Author

nvkevlu commented Jan 15, 2025

/build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YuanTingHsieh YuanTingHsieh YuanTingHsieh left review comments

@yanchengnv yanchengnv yanchengnv requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nvkevlu @YuanTingHsieh @yanchengnv