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

[show]: Remove default commands; add missing docstrings #121

Merged
merged 2 commits into from
Oct 5, 2017
Merged

[show]: Remove default commands; add missing docstrings #121

merged 2 commits into from
Oct 5, 2017

Conversation

jleveque
Copy link
Contributor

@jleveque jleveque commented Oct 5, 2017

No description provided.

@jleveque jleveque self-assigned this Oct 5, 2017
@jleveque jleveque requested a review from lguohan October 5, 2017 19:59
@jleveque
Copy link
Contributor Author

jleveque commented Oct 5, 2017

@rodnymolina: Please review.

Copy link
Contributor

@rodnymolina rodnymolina left a comment

Choose a reason for hiding this comment

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

Joe, couple of questions:

  • Your goal with these changes is to bring back the 'tabs' functionality, right?

  • If we are not using 'default' feature, why are we still making use of AliasedGroup? Shouldn't we simply rely on Click.group decorator instead?

@jleveque
Copy link
Contributor Author

jleveque commented Oct 5, 2017

@rodnymolina: This will fix the tab completion on the show interfaces transceiver and show processes commands you added. Also, you forgot to add the bash completion files for your new utilities to setup.py, so I'm trying to copy all of them using a glob here to prevent this in the future.

However, there is a bigger issue that was introduced with the .deb -> .whl change that breaks all tab completion. So, I plan to try to enhance our build system in order to install sonic-utilities as a .deb package again. The .whl package has more limitations than the .deb package and copying files (i.e., our bash_completion files) outside of the dist-packages directory is impossible.

@jleveque jleveque merged commit 52dd13e into sonic-net:master Oct 5, 2017
@jleveque jleveque deleted the fixes branch October 5, 2017 23:04
@jleveque
Copy link
Contributor Author

jleveque commented Oct 5, 2017

@rodnymolina: With regards to AliasedGroup: We should no longer rely on DefaultGroup -- AliasedGroup should extend Click.group directly, OR we need to fix/write our own DefaultGroup that doesn't break tab completion.

vdahiya12 pushed a commit to vdahiya12/sonic-utilities that referenced this pull request Jul 23, 2021
Fixes the following crash introduced by sonic-net/sonic-platform-daemons#102:

```
01:33:00  ______________________ test_updater_thermal_check_min_max ______________________
01:33:00  
01:33:00      def test_updater_thermal_check_min_max():
01:33:00          chassis = MockChassis()
01:33:00      
01:33:00          thermal = MockThermal()
01:33:00          chassis.get_all_thermals().append(thermal)
01:33:00      
01:33:00          chassis.set_modular_chassis(True)
01:33:00          chassis.set_my_slot(1)
01:33:00          temperature_updater = TemperatureUpdater(SYSLOG_IDENTIFIER, chassis)
01:33:00      
01:33:00          temperature_updater.update()
01:33:00          slot_dict = temperature_updater.chassis_table.get('Thermal 1')
01:33:00  >       assert slot_dict['minimum_temperature'] == str(thermal.get_minimum_recorded())
01:33:00  E       TypeError: 'NoneType' object has no attribute '__getitem__'
01:33:00  
01:33:00  tests/test_thermalctld.py:341: TypeError
```

Signed-off-by: Petro Bratash <petrox.bratash@intel.com>

Signed-off-by: Petro Bratash <petrox.bratash@intel.com>
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 28, 2022
[EEPROM] Add new function part_number_str to TlvInfoDecoder (sonic-net#121)

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
mihirpat1 pushed a commit to mihirpat1/sonic-utilities that referenced this pull request Sep 15, 2023
…t#121)

Add a new function to TlvInfoDecoder which can return device part number as a string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants