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

feat(framework) Add flwr log #3577

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

feat(framework) Add flwr log #3577

wants to merge 98 commits into from

Conversation

chongshenng
Copy link
Contributor

@chongshenng chongshenng commented Jun 11, 2024

Issue

Description

With the SuperExec service, the ServerApp logs are not easily accessible. This CLI enables a request to be sent to the service.

Related issues/PRs

The companion PR below adds log capturing to the SuperExec service. This PR can be merged independently of it.

Blocks:

Proposal

Explanation

flwr log sends a requests to the SuperExec service to get the log stream and receives a response from it. The response will be handled in two ways:

  1. Run flwr log <RUN_ID> <DIRECTORY> <FEDERATION_NAME>. The default is stream=True (--stream). This streams the logs and periodically refreshes the gRPC connection.
  2. Run flwr log <RUN_ID> <DIRECTORY> <FEDERATION_NAME> --show. This prints the logs and closes the connection gracefully. Every subsequent execution of this command always prints the logs from the beginning of the run.

Checklist

  • Implement proposed change
  • Make CI checks pass
  • Ping maintainers on Slack (channel #contributions)

Any other comments?

@chongshenng chongshenng self-assigned this Jun 11, 2024
@chongshenng chongshenng marked this pull request as ready for review June 11, 2024 20:39
src/py/flwr/cli/log.py Outdated Show resolved Hide resolved
src/py/flwr/cli/log.py Outdated Show resolved Hide resolved
src/py/flwr/cli/log.py Outdated Show resolved Hide resolved
src/py/flwr/cli/log.py Show resolved Hide resolved
src/py/flwr/cli/log.py Outdated Show resolved Hide resolved
src/py/flwr/cli/log.py Show resolved Hide resolved
src/py/flwr/cli/log.py Outdated Show resolved Hide resolved
src/py/flwr/cli/log.py Show resolved Hide resolved
src/py/flwr/cli/log.py Show resolved Hide resolved
@chongshenng
Copy link
Contributor Author

@jafermarq About your comments here, we need to refactor flwr log and flwr run together soon since they share a lot of similar code. It's substantial work worthy of another PR.

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

Successfully merging this pull request may close these issues.

8 participants