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

Fixed the args.log_config setting. #86

Merged
merged 2 commits into from
Jan 5, 2022
Merged

Fixed the args.log_config setting. #86

merged 2 commits into from
Jan 5, 2022

Conversation

yhwen
Copy link
Collaborator

@yhwen yhwen commented Dec 31, 2021

No description provided.

@yhwen yhwen linked an issue Dec 31, 2021 that may be closed by this pull request
The args.log_config is not set up properly #87
Closed
nvkevlu
nvkevlu previously approved these changes Jan 3, 2022
View reviewed changes
IsaacYangSLA
IsaacYangSLA approved these changes Jan 4, 2022
View reviewed changes
Copy link
Collaborator

@IsaacYangSLA IsaacYangSLA left a comment

Choose a reason for hiding this comment

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

The updated portion is good. However, it seemed we piggybacked some information into args attributes while we can use another variable.

@YuanTingHsieh
Copy link
Collaborator

Yeah, if we need this log_config in args then the current code is good. (args.log_config)
If we don't need it in args, then we just use a new variable. (for example: app_log_config)

@yhwen
Copy link
Collaborator Author

yhwen commented Jan 5, 2022

Yeah, if we need this log_config in args then the current code is good. (args.log_config) If we don't need it in args, then we just use a new variable. (for example: app_log_config)

args.log_config has been used outside as a variable to hold the log config.

@IsaacYangSLA IsaacYangSLA merged commit 5316ed4 into NVIDIA:main Jan 5, 2022
@yhwen yhwen deleted the fix_log_config branch April 28, 2022 14:18
holgerroth pushed a commit to holgerroth/NVFlare that referenced this pull request Dec 4, 2023
@yhwen
Fixed the args.log_config setting. (NVIDIA#86)
0e21b5d 
* Fixed the args.log_config setting.

* Enhanced to allow FL app config/log.config to overwrite the settings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IsaacYangSLA IsaacYangSLA IsaacYangSLA approved these changes

@nvkevlu nvkevlu nvkevlu approved these changes

@yanchengnv yanchengnv Awaiting requested review from yanchengnv

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

Successfully merging this pull request may close these issues.

The args.log_config is not set up properly
4 participants
@yhwen @YuanTingHsieh @IsaacYangSLA @nvkevlu