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

construct_deep_url() #53475

Open
wants to merge 29 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@RobW3LGA
Copy link

RobW3LGA commented Mar 7, 2019

SUMMARY
  • Adds the ability to create deeper objects to an ACI fabric and can potentially replace the need for the aci_rest module
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • Modifies aci.py with construct_deep_url() and supporting functions
ADDITIONAL INFORMATION
  • This function can also replace shallow ACI objects with a slight module change

  • Example:

aci.construct_url(
        root_class=dict(
            aci_class='fvTenant',
            aci_rn='tn-{0}'.format(tenant),
            module_object=tenant,
            target_filter={'name': tenant},
        ),
        subclass_1=dict(
            aci_class='fvBD',
            aci_rn='BD-{0}'.format(bd),
            module_object=bd,
            target_filter={'name': bd},
        ),
        child_classes=['fvRsCtx', 'fvRsIgmpsn', 'fvRsBDToNdP', 'fvRsBdToEpRet'],
    )
  • vs:
aci.construct_deep_url(
        parent_objects=[
            dict(
                parent_class='uni',
                aci_class='fvTenant',
                aci_rn='tn-{0}'.format(tenant),
                target_filter={'name': tenant},
                module_object=tenant
            ),
        ],
        target_object=dict(
            parent_class='fvTenant',
            aci_class='fvBD',
            aci_rn='BD-{0}'.format(bd),
            target_filter={'name': bd},
            module_object=bd
        ),
        child_classes=['fvRsCtx', 'fvRsIgmpsn', 'fvRsBDToNdP', 'fvRsBdToEpRet']
    )

RobW3LGA added some commits Mar 7, 2019

Updates aci.py with the ability to add ACI objects to any depth
Changes start at line 411 (construct_deep_url() and supporting functions). One minor change to line 633 (the original construct_url()) to provide for testability:  ...join(sorted(self.child_classes)) vs ...join(self.child_classes)

I am also attaching two test files. One characterizing the existing construct_url() and the matching test set for construct_deep_url() to support my efforts and proof of parity
Two PyTest files to support construct_deep_url
These two files provide testing parity, one characterizing the original construct_url() function and the other proofing construct_deep_url(). The ...deep_url.py test file goes five layers deep to provide better validation for the function
@ansibot

This comment has been minimized.

Correcting previous upload to incorrect folder
These two files provide testing parity, one characterizing the original construct_url() function and the other proofing construct_deep_url(). The ...deep_url.py test file goes five layers deep to provide better validation for the function
@RobW3LGA

This comment has been minimized.

Copy link
Author

RobW3LGA commented on 13b7af7 Mar 7, 2019

Please cancel this pull request. I have re-uploaded these to the correct folder per Matt Clay (thanks for the help, Matt)

RobW3LGA added some commits Mar 7, 2019

RobW3LGA added some commits Mar 7, 2019

Correcting file names per Matt Clay
@mattclay Thanks again for your continued guidance and patience. Please cancel the previous (incorrect) request
@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 7, 2019

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 2 errors:

test/units/module_utils/network/aci/test_aci_construct_url.py:3:1: SyntaxError: 
test/units/module_utils/network/aci/test_aci_deep_url.py:3:1: SyntaxError:

The test ansible-test sanity --test line-endings [explain] failed with 2 errors:

test/units/module_utils/network/aci/test_aci_construct_url.py:0:0: use "\n" for line endings instead of "\r\n"
test/units/module_utils/network/aci/test_aci_deep_url.py:0:0: use "\n" for line endings instead of "\r\n"

The test ansible-test sanity --test pep8 [explain] failed with 23 errors:

lib/ansible/module_utils/network/aci/aci.py:558:161: E501 line too long (188 > 160 characters)
test/units/module_utils/network/aci/test_aci_construct_url.py:540:161: E501 line too long (201 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:55:161: E501 line too long (203 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:647:161: E501 line too long (201 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:883:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:931:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:979:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1120:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1122:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1123:161: E501 line too long (218 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:1169:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1171:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1172:161: E501 line too long (227 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:1219:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1407:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1409:9: E265 block comment should start with '# '
test/units/module_utils/network/aci/test_aci_deep_url.py:1410:161: E501 line too long (218 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:1529:161: E501 line too long (189 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:1585:161: E501 line too long (189 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:1921:161: E501 line too long (167 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:1976:161: E501 line too long (167 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:2089:161: E501 line too long (164 > 160 characters)
test/units/module_utils/network/aci/test_aci_deep_url.py:2145:161: E501 line too long (174 > 160 characters)

The test ansible-test sanity --test shebang [explain] failed with 2 errors:

test/units/module_utils/network/aci/test_aci_construct_url.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'
test/units/module_utils/network/aci/test_aci_deep_url.py:1:1: unexpected non-module shebang: b'#!/usr/bin/python'

click here for bot help

RobW3LGA added some commits Mar 7, 2019

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 7, 2019

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 2 errors:

test/units/module_utils/network/aci/test_aci_construct_url.py:5:1: SyntaxError: 
test/units/module_utils/network/aci/test_aci_deep_url.py:5:1: SyntaxError:

The test ansible-test sanity --test line-endings [explain] failed with 2 errors:

test/units/module_utils/network/aci/test_aci_construct_url.py:0:0: use "\n" for line endings instead of "\r\n"
test/units/module_utils/network/aci/test_aci_deep_url.py:0:0: use "\n" for line endings instead of "\r\n"

click here for bot help

RobW3LGA added some commits Mar 8, 2019

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 8, 2019

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 2 errors:

test/units/module_utils/network/aci/test_aci_construct_url.py:2:1: SyntaxError: 
test/units/module_utils/network/aci/test_aci_deep_url.py:2:1: SyntaxError:

The test ansible-test sanity --test line-endings [explain] failed with 2 errors:

test/units/module_utils/network/aci/test_aci_construct_url.py:0:0: use "\n" for line endings instead of "\r\n"
test/units/module_utils/network/aci/test_aci_deep_url.py:0:0: use "\n" for line endings instead of "\r\n"

click here for bot help

RobW3LGA added some commits Mar 8, 2019

Pro Tip: Convert from 'CRLF' to 'LF' in VSCode
It's on the status bar to the right
Added two mocks to support testing
I could not find where to place fakes/mocks, so please let me know if the current location is incorrect
@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 8, 2019

The test ansible-test sanity --test compile --python 2.6 [explain] failed with 1 error:

test/units/module_utils/network/aci/mock_basic.py:2:1: SyntaxError:

The test ansible-test sanity --test line-endings [explain] failed with 1 error:

test/units/module_utils/network/aci/mock_basic.py:0:0: use "\n" for line endings instead of "\r\n"

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

test/units/module_utils/network/aci/mock_basic.py:3:1: E265 block comment should start with '# '

click here for bot help

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Mar 8, 2019

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

test/units/module_utils/network/aci/mock_basic.py:50:0: missing-final-newline Final newline missing
test/units/module_utils/network/aci/mock_basic.py:50:0: mixed-line-endings Mixed line endings LF and CRLF

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

test/units/module_utils/network/aci/mock_basic.py:50:28: W292 no newline at end of file

click here for bot help

RobW3LGA added some commits Mar 8, 2019

Added last blank line to mock_basic.py
To pass sanity test

@ansibot ansibot removed the support:core label Mar 8, 2019

Merge pull request #1 from ansible/devel
Re-sync forked repo
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 8, 2019

@RobW3LGA this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@@ -446,7 +630,7 @@ def construct_url(self, root_class, subclass_1=None, subclass_2=None, subclass_3

if self.child_classes:
# Append child_classes to filter_string if filter string is empty
self.update_qs({'rsp-subtree': 'full', 'rsp-subtree-class': ','.join(self.child_classes)})
self.update_qs({'rsp-subtree': 'full', 'rsp-subtree-class': ','.join(sorted(self.child_classes))})

This comment has been minimized.

@dagwieers

dagwieers Mar 11, 2019

Member

Why sorted?

This comment has been minimized.

@RobW3LGA

RobW3LGA Mar 12, 2019

Author

When under test, self.child_classes appears to randomize the response when .join() is called (the unit test randomly fails). Sorted() eliminates the randomization and allows the test to pass consistently. My next PR will allow aci.py to be testable and I will be able to demonstrate this with the unit tests I have ready to upload.

@ansibot ansibot added the stale_ci label Mar 20, 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.