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

Support hierarchical keys #12

Merged
merged 4 commits into from
Sep 8, 2017
Merged

Support hierarchical keys #12

merged 4 commits into from
Sep 8, 2017

Conversation

taoyl-ms
Copy link
Contributor

  1. Use | instead of : for separator in redis key, and support hierarchical multiple keys.
  2. Support object with no attribute (data == {}) with a "NULL": "NULL" placeholder.
  3. set_entry now support delete entry with data == None.

@taoyl-ms taoyl-ms requested a review from lguohan August 29, 2017 22:16
@msftclas
Copy link

msftclas commented Aug 29, 2017 •
edited by taoyl-ms
Loading

@taoyl-ms,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot #Closed

lguohan
lguohan reviewed Sep 1, 2017
View reviewed changes
src/swsssdk/configdb.py Outdated
@@ -84,7 +85,7 @@ def listen(self):
if item['type'] == 'pmessage':
key = item['channel'].split(':', 1)[1]
try:
(table, row) = key.split(':', 1)
(table, row) = key.split('|', 1)
Copy link
Contributor

@lguohan lguohan Sep 1, 2017 •
edited by taoyl-ms
Loading

Choose a reason for hiding this comment

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

'|' [](start = 45, length = 3)

can you use a variable? #Resolved

@lguohan
Copy link
Contributor

lguohan commented Sep 1, 2017 •
edited by taoyl-ms
Loading

as a general comment, we need unit test for this configdb class. #WontFix

lguohan
lguohan reviewed Sep 1, 2017
View reviewed changes
src/swsssdk/configdb.py Outdated
@@ -117,16 +123,30 @@ def __typed_to_raw(self, typed_data):
raw_data[key] = value
return raw_data

@staticmethod
def serialize_key(key):
string_types = str if sys.version_info[0] == 3 else basestring
Copy link
Contributor

@lguohan lguohan Sep 1, 2017 •
edited by taoyl-ms
Loading

Choose a reason for hiding this comment

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

string_types = str if sys.version_info[0] == 3 else basestring [](start = 8, length = 62)

in general, I found such shortcut conditional not quite readable, I prefer traditional if else condition. #Resolved

lguohan
lguohan reviewed Sep 1, 2017
View reviewed changes
src/swsssdk/configdb.py Outdated
@staticmethod
def deserialize_key(key):
tokens = key.split('|')
return tuple(tokens) if len(tokens) > 1 else tokens[0]
Copy link
Contributor

@lguohan lguohan Sep 1, 2017 •
edited by taoyl-ms
Loading

Choose a reason for hiding this comment

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

eturn tuple(tokens) if len(tokens) > 1 else tokens[0] [](start = 9, length = 53)

not readable, I think we should try to avoid to keep the code more readable. #Resolved

@lguohan
Copy link
Contributor

lguohan commented Sep 1, 2017 •
edited by taoyl-ms
Loading 

        { 'row_key': {'column_key': 'value', ...}, ...}

can you add hierachical key examples? this will be one of most heavily used library, should be heavily documented. #Resolved

Refers to: src/swsssdk/configdb.py:171 in f975305. [](commit_id = f975305, deletion_comment = False)

@taoyl-ms
Copy link
Contributor Author

taoyl-ms commented Sep 8, 2017

Agree. Will add in later PRs.

In reply to: 326614104 [](ancestors = 326614104)

lguohan
lguohan approved these changes Sep 8, 2017
View reviewed changes
Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

:shipit:

@taoyl-ms taoyl-ms mentioned this pull request Sep 8, 2017
Merged
@taoyl-ms taoyl-ms merged commit 7f8e7c5 into sonic-net:master Sep 8, 2017
praveen-li pushed a commit to praveen-li/sonic-py-swsssdk that referenced this pull request May 4, 2021
Merge remote-tracking branch 'msft_github/master'
18e5d78 
* msft_github/master:
  Commond utility functions for bridge/port mapping (sonic-net#14)
  [configdb] Support hierarchical keys (sonic-net#12)
  Add attribute retry_on for configdb connection (sonic-net#11)
qiluo-msft pushed a commit that referenced this pull request Mar 21, 2022
@Junchao-Mellanox
[port_util] Fix issue: port_util.get_vlan_interface_oid_map should no…
96c0590 
…t raise exception when DB has not RIF data (#117)

**Why I did this?**

port_util.get_vlan_interface_oid_map would raise exception when there is no RIF configured, and it would causes snmpagent not work. This PR is to fix it.

```
#012Traceback (most recent call last):
#012File "/usr/local/lib/python3.7/dist-packages/ax_interface/mib.py", line 37, in start
#12    self.reinit_data()#12  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/ietf/rfc1213.py", line 233, in reinit_data
#12    self.vlan_oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_vlan_tables, self.db_conn)
#12  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/__init__.py", line 651, in get_sync_d_from_all_namespace
#12    ns_tuple = per_namespace_func(db_conn)
#12  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/__init__.py", line 341, in init_sync_d_vlan_tables
#12    vlan_name_map = port_util.get_vlan_interface_oid_map(db_conn)
#12  File "/usr/local/lib/python3.7/dist-packages/swsssdk/port_util.py", line 167, in get_vlan_interface_oid_map
#12    rif_name_map = db.get_all('COUNTERS_DB', 'COUNTERS_RIF_NAME_MAP', blocking=True)
#12  File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1751, in get_all
#12    return dict(super(SonicV2Connector, self).get_all(db_name, _hash, blocking))
#12  File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1708, in get_all
#12    return _swsscommon.SonicV2Connector_Native_get_all(self, db_name, _hash, blocking)
#012RuntimeError: Key '{COUNTERS_RIF_NAME_MAP}' unavailable in database '{COUNTERS_DB}'
```

**How I did this:**
check redis key exists first before calling get_all.
judyjoseph pushed a commit that referenced this pull request Mar 28, 2022
@Junchao-Mellanox @judyjoseph
[port_util] Fix issue: port_util.get_vlan_interface_oid_map should no…
d48f750 
…t raise exception when DB has not RIF data (#117)

**Why I did this?**

port_util.get_vlan_interface_oid_map would raise exception when there is no RIF configured, and it would causes snmpagent not work. This PR is to fix it.

```
#012Traceback (most recent call last):
#012File "/usr/local/lib/python3.7/dist-packages/ax_interface/mib.py", line 37, in start
#12    self.reinit_data()#12  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/ietf/rfc1213.py", line 233, in reinit_data
#12    self.vlan_oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_vlan_tables, self.db_conn)
#12  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/__init__.py", line 651, in get_sync_d_from_all_namespace
#12    ns_tuple = per_namespace_func(db_conn)
#12  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/__init__.py", line 341, in init_sync_d_vlan_tables
#12    vlan_name_map = port_util.get_vlan_interface_oid_map(db_conn)
#12  File "/usr/local/lib/python3.7/dist-packages/swsssdk/port_util.py", line 167, in get_vlan_interface_oid_map
#12    rif_name_map = db.get_all('COUNTERS_DB', 'COUNTERS_RIF_NAME_MAP', blocking=True)
#12  File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1751, in get_all
#12    return dict(super(SonicV2Connector, self).get_all(db_name, _hash, blocking))
#12  File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1708, in get_all
#12    return _swsscommon.SonicV2Connector_Native_get_all(self, db_name, _hash, blocking)
#012RuntimeError: Key '{COUNTERS_RIF_NAME_MAP}' unavailable in database '{COUNTERS_DB}'
```

**How I did this:**
check redis key exists first before calling get_all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lguohan lguohan lguohan approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@taoyl-ms @msftclas @lguohan