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

Adding New Model onyx_qos for Configuring QoS on Onyx Switches #55127

Open
wants to merge 7 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@anasbadaha
Copy link
Contributor

commented Apr 11, 2019

Signed-off-by: Anas Badaha anasb@mellanox.com

SUMMARY

Adding New Model onyx_qos for Configuring QoS on Onyx Switches

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

lib/ansible/modules/network/onyx/onyx_qos.py
test/units/modules/network/onyx/fixtures/show_qos_interface_ethernet.cfg
test/units/modules/network/onyx/test_onyx_qos.py

ADDITIONAL INFORMATION
ansible 2.8.0a1.post0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /.autodirect/mtrswgwork/anasb/ansible_dev8/lib/ansible
  executable location = /.autodirect/mtrswgwork/anasb/ansible_dev8/bin/ansible
  python version = 2.7.5 (default, Jul 13 2018, 13:06:57) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

Adding New Model onyx_qos for Configuring QoS on Onyx Switches
Signed-off-by: Anas Badaha <anasb@mellanox.com>
@ansibot

This comment has been minimized.

Fix Pep8 Failures in onyx_qos
Signed-off-by: Anas Badaha <anasb@mellanox.com>
Fix Pep8 Failures phase 2
Signed-off-by: Anas Badaha <anasb@mellanox.com>
@anasbadaha

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

Hi @samerd,

Please take a look on this PR and approve it if you don't have farther comments.

Thanks

@ansibot ansibot added core_review and removed needs_revision labels Apr 11, 2019

@samerd
Copy link
Contributor

left a comment

Thanks Anas.
excellent module.
However, I have few minor comments:
1- rewrite_dscp and rewrite_pcp, should be both disabled by default
2- regarding the "no" commands, although your commands are working, I prefer to put the no before the qos, instead before the interface
NO_REWRITE_PCP_CMD = "interface {0} {1} no qos rewrite pcp"
NO_REWRITE_DSCP_CMD = "interface {0} {1} no qos rewrite dscp"

anasbadaha added some commits Apr 14, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 14, 2019

@anasbadaha this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

anasbadaha added some commits Apr 14, 2019

Fix Shippable Comments Phase 3
Signed-off-by: Anas Badaha <anasb@mellanox.com>
Fix Current Version 2.9
Signed-off-by: Anas Badaha <anasb@mellanox.com>
@anasbadaha

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

Hi @samerd ,

I have fixed all your comments, please approve if it's OK for you.

Thanks

@samerd

samerd approved these changes Apr 15, 2019

@ansibot ansibot removed the needs_revision label Apr 15, 2019

@samerd

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

@justjais
please review and merge if OK with you

@anasbadaha

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2019

Hi @justjais,

Can you please merge this PR if it is OK for you?

Thanks

@ansibot ansibot added the stale_ci label Apr 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.