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

[SCB-638] Config model mechanism #1087

Merged
merged 2 commits into from Feb 13, 2019
Merged

Conversation

wujimin
Copy link
Contributor

@wujimin wujimin commented Feb 3, 2019

support get config value by priority or inject priority value to model

  @InjectProperties(prefix = "root")
  public static class ConfigWithAnnotation {
    @InjectProperty(prefix = "override", keys = {"high", "low"})
    public String strValue;

    @InjectProperty(keys = "${key}.value")
    public int intValue;

    @InjectProperty(keys = "${list-1}.a.${list-2}.b")
    public long longValue;

    @InjectProperty(keys = "${full-list}")
    public float floatValue;
}

@coveralls
Copy link

coveralls commented Feb 3, 2019

Coverage Status

Coverage increased (+0.09%) to 86.032% when pulling 94b975c on wujimin:config-model-mechanism into 9040ec3 on apache:master.


@SuppressWarnings("unchecked")
static <T> Property<T> getLongProperty(String propName, T defaultValue) {
return (Property<T>) new DynamicLongProperty(propName, (Long) defaultValue);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can remove the annotation @SuppressWarnings("unchecked") like below?

  static Property<Long> getLongProperty(String propName, Long defaultValue) {
    return new DynamicLongProperty(propName, defaultValue);
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// l1-1.a.l2-1.b
// l1-2.a.l2-1.b
// l1-1.a.l2-2.b
// l1-2.a.l2-2.b
Copy link
Member

Choose a reason for hiding this comment

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

It seems that the backward placeholders have a higher priority. Does this meets your expectation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's my expectation, but it's not correct
i'll fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@wujimin wujimin merged commit 5d46c29 into apache:master Feb 13, 2019
@wujimin wujimin deleted the config-model-mechanism branch March 14, 2019 13:59
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.

None yet

4 participants