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 a problem when set flags in all module. #713

Merged
merged 3 commits into from
Aug 13, 2019
Merged

Conversation

critical27
Copy link
Contributor

@critical27 critical27 commented Aug 1, 2019

  1. Fix when we want to update variables in all module, it would fail if one config is immutable. For now, it will update all mutable variables and ignore the immutable variables.
  2. Delete all load_config_interval_secs (it is used in early implement version), just keep same speed with load_data_interval_secs.

TODO: Maybe we will organize all config into a separate file, and avoid hard code, and config manager will load it and try to register on meta.

@nebula-community-bot
Copy link
Member

Unit testing passed.

1 similar comment
@nebula-community-bot
Copy link
Member

Unit testing passed.

@critical27 critical27 changed the title fix set variables would fail, delete all load_config_interval_secs organize out gflags into a file, fix a problem when set flags in all module. Aug 2, 2019
@critical27 critical27 added in progress and removed ready-for-testing PR: ready for the CI test labels Aug 2, 2019
@nebula-community-bot
Copy link
Member

Unit testing passed.

@critical27 critical27 changed the title organize out gflags into a file, fix a problem when set flags in all module. fix a problem when set flags in all module. Aug 8, 2019
@sherman-the-tank
Copy link
Member

TODO: Maybe we will organize all config into a separate file, and avoid hard code, and config manager will load it and try to register on meta.

First, can we add this TODO into the code? Messages here intend to be lost when the PR is merged

@critical27
Copy link
Contributor Author

critical27 commented Aug 13, 2019

Unit testing passed.

Yeah, I will open an issue to record.

Copy link
Contributor

@darionyaphet darionyaphet left a comment

Choose a reason for hiding this comment

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

LGTM

@nebula-community-bot
Copy link
Member

Unit testing passed.

@nebula-community-bot
Copy link
Member

Unit testing passed.

@dutor dutor merged commit 1e9bb68 into vesoft-inc:master Aug 13, 2019
@critical27 critical27 deleted the fix branch August 14, 2019 13:13
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Mar 21, 2022
Co-authored-by: liwenhui-soul <38217397+liwenhui-soul@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants