Skip to content

Improve decorator to support profile config.#684

Open
liuh-80 wants to merge 43 commits intosonic-net:masterfrom
liuh-80:dev/liuh/add-decorator
Open

Improve decorator to support profile config.#684
liuh-80 wants to merge 43 commits intosonic-net:masterfrom
liuh-80:dev/liuh/add-decorator

Conversation

@liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Sep 9, 2022

Why I did it

Support sonic-swss-common read profile config.

How I did it

Improve decorator to support profile config.

How to verify it

Add new UT to cover new decorate code.
Pass all UT and E2E test cases.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Improve decorator to support profile config.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@liuh-80
Copy link
Contributor Author

liuh-80 commented Sep 9, 2022

This PR depends on 3 other PRs merge first to pass build validation:

Repo PR Title State
sonic-swss-common Add ProfileProvider class to support read profile config from PROFILE_DB. GitHub issue/pull request detail
sonic-swss-common Modify azure pipeline to install libyang in azure pipeline steps. GitHub issue/pull request detail
sonic-swss-common Load Yang model default value to DefaultValueProvider. GitHub issue/pull request detail

@liuh-80 liuh-80 marked this pull request as ready for review September 9, 2022 05:16
@liuh-80 liuh-80 requested a review from qiluo-msft September 9, 2022 05:16
@liuh-80
Copy link
Contributor Author

liuh-80 commented Oct 17, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 changed the title Add decorator for Yang default value and profile config. Improve decorator to support profile config. Nov 22, 2022
std::shared_ptr<DefaultValueProvider> m_defaultValueProvider;

/* Handle SubscriberStateTable unknown pattern */
void onPopUnknownPattern(RedisMessage& message, std::deque<KeyOpFieldsValuesTuple> &vkco) override;
Copy link
Contributor

Choose a reason for hiding this comment

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

override

override means override a function which already implemented in base class.

If this is a new function in this class and its subclass, you instead use virtual.

bool hget(const std::string &key, const std::string &field, std::string &value) override;

/* Get all the keys from the table */
void getKeys(std::vector<std::string> &keys) override;
Copy link
Contributor

Choose a reason for hiding this comment

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

override

base class Table is not virtual.

const std::string &op = "",
const std::string &prefix = EMPTY_PREFIX) override;

/* Unhide override 'set' methods in base class */
Copy link
Contributor

@qiluo-msft qiluo-msft Jun 20, 2023

Choose a reason for hiding this comment

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

override

Do you mean overload? If overridden already, how to unhide?

}

protected:
virtual void onPopUnknownPattern(RedisMessage& message, std::deque<KeyOpFieldsValuesTuple> &vkco);
Copy link
Contributor

Choose a reason for hiding this comment

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

virtual

Do you really need it as virtual? It is required if user have a base class pointer or reference to an instance of a subclass.

self.default_value_provider = DefaultValueProvider()
# helper methods for append default values to result.
# helper methods for append profile and default values to result.
def _append_profile(self, result):
Copy link
Contributor

Choose a reason for hiding this comment

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

_append_profile

Do you want to create another Decorator? this new feature is not related to yang default value.

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.

2 participants