added autoRefresh option for regular login (non-websocket connections) #34

kprav33n · 2018-12-23T21:14:58Z

This should resolve #16

kprav33n · 2018-12-23T21:19:25Z

Thanks for the contribution. We'll also have to address the CI issues identified circleci, travis-ci, and codacy before we can merge this PR.

…hub..

auto refreshes them

codecov · 2018-12-24T23:54:11Z

Codecov Report

Merging #34 into master will increase coverage by 2.18%.
The diff coverage is 78.03%.

@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
+ Coverage   65.15%   67.34%   +2.18%     
==========================================
  Files           8        8              
  Lines         924     1087     +163     
==========================================
+ Hits          602      732     +130     
- Misses        322      355      +33

Impacted Files	Coverage Δ
pyaci/core.py	`72.76% <78.03%> (+1.87%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b43fe90...f335c39. Read the comment docs.

toravir · 2018-12-25T02:22:57Z

Fixes both #16 and #17 - and addressed the first set of code review comments.

toravir · 2018-12-25T02:26:29Z

@kprav33n pls review the websocket auto refresh and test code. autoRefresh param during Login() controls both autoRefresh of Login Sessions AND subscriptions - if we need different knobs for both separately, can do it too - my guess is that single knob is simple.

kprav33n

Thanks for addressing the comments. Some minor style issues suggested. We can take care of some now, and others later.

kprav33n · 2018-12-26T06:59:33Z

pyaci/core.py


+#Web Socket Statuses


Suggested change

#Web Socket Statuses

# Web Socket Statuses

kprav33n · 2018-12-26T07:00:25Z

pyaci/core.py


+#Web Socket Statuses
+WS_OPENING = 'Websocket Opening...'
+WS_OPEN    = 'Websocket Open.'


Suggested change

WS_OPEN = 'Websocket Open.'

WS_OPENED = 'Websocket Opened.'

kprav33n · 2018-12-26T07:00:50Z

pyaci/core.py


+#Web Socket Statuses
+WS_OPENING = 'Websocket Opening...'


Suggested change

WS_OPENING = 'Websocket Opening...'

WS_OPENING = 'Websocket Opening.'

kprav33n · 2018-12-26T07:03:12Z

pyaci/core.py

+        self._autoRefresh = False
+        self._autoRefreshThread = None
+        self._login = {}
+        self._autotest = False


I'm not thrilled to have test specific handling in the product code. We'll leave this for now, and discuss later how to remove it.

kprav33n · 2018-12-26T07:03:45Z

pyaci/core.py

+        if 'APIC-cookie' in self._rootApi()._session.cookies :
+            token = self._rootApi()._session.cookies['APIC-cookie']
+        else:
+            raise Exception('APIC-cookie NOT found.. Make sure you have logged in.')


We should have specific exceptions/errors thrown instead of the blanket Exception. We can handle this later.

kprav33n · 2018-12-26T07:06:35Z

pyaci/core.py

@@ -698,12 +731,102 @@ def _spawnChildFromAttributes(self, className, **attributes):
        return self._spawnChildFromRn(className, rn)


+class autoRefreshThread(threading.Thread):
+    def __init__(self, rootApi):
+        super(autoRefreshThread, self).__init__()


Suggested change

super(autoRefreshThread, self).__init__()

super(AutoRefreshThread, self).__init__()

kprav33n · 2018-12-26T07:07:26Z

pyaci/core.py

+
+    def run(self):
+        logger.debug('arThread: Starting up...')
+        REFRESH_BEFORE = 60  #approx - this many seconds before expiry, do refresh


Suggested change

REFRESH_BEFORE = 60 #approx - this many seconds before expiry, do refresh

REFRESH_BEFORE = 60 # approx - this many seconds before expiry, do refresh

kprav33n · 2018-12-26T07:07:36Z

pyaci/core.py

+    def run(self):
+        logger.debug('arThread: Starting up...')
+        REFRESH_BEFORE = 60  #approx - this many seconds before expiry, do refresh
+        CHECK_INTERVAL = 10  #how long to sleep before waking to check


Suggested change

CHECK_INTERVAL = 10 #how long to sleep before waking to check

CHECK_INTERVAL = 10 # how long to sleep before waking to check

kprav33n · 2018-12-26T07:07:47Z

pyaci/core.py

+        logger.debug('arThread: Starting up...')
+        REFRESH_BEFORE = 60  #approx - this many seconds before expiry, do refresh
+        CHECK_INTERVAL = 10  #how long to sleep before waking to check
+        WS_REFRESH_INT = 40  #approx - this many seconds before subscription refresh


Suggested change

WS_REFRESH_INT = 40 #approx - this many seconds before subscription refresh

WS_REFRESH_INT = 40 # approx - this many seconds before subscription refresh

kprav33n · 2018-12-26T07:08:44Z

pyaci/core.py

+                                                               int(doc['imdata']['aaaLogin']['@refreshTimeoutSeconds'])
+                logger.debug(root._login)
+                if root._autoRefresh:
+                    arThread = autoRefreshThread(root)


Suggested change

arThread = autoRefreshThread(root)

arThread = AutoRefreshThread(root)

toravir · 2018-12-26T19:19:52Z

@kprav33n addressed code review comments - removed the knob for testing in the production code by restructuring the code.

only thing to discuss and do later is: exceptions from pyaci
another todo is for a way to run all CI, codacy, codecov tests locally (before the commit)

kprav33n · 2018-12-27T01:05:13Z

Thank you for meticulously taking care of all review comments. Appreciate the contribution. Merged.

added autoRefresh option for regular login (non-websocket connections)

18dd62e

Ravi R added 5 commits December 24, 2018 10:54

Updated code review comments.

8a3d794

Docker file change to allow using local WS or pick up latest from git…

abddefd

…hub..

autoRefresh knob also checks if any subscriptions are present, if so

d1032ea

auto refreshes them

fixed test failures..

1b13dc3

Fixed more..

e9955b2

Ravi R added 11 commits December 24, 2018 16:25

added tests for code coverage etc..

50d8b0b

fixed more errors..

1fd466f

more minor changes in testcode.

e63c0e2

fixed typos..

c28a958

removed an extra comma...

8fda012

added more tests to increase code coverage

8002bcd

auto refresh tests.

c2072ca

missed import

3c31103

minor changes..

76ac963

fixing reverting change to run() method of autoRefreshThread...

c71bcc3

bypassing docker lint error for pip install

c6c501c

kprav33n commented Dec 26, 2018

View reviewed changes

Ravi R added 3 commits December 26, 2018 11:10

addressed code review comments

a565f04

few more fixes per comments

d89b539

fixed typo in test code..

f335c39

kprav33n merged commit c462454 into datacenter:master Dec 27, 2018

kprav33n mentioned this pull request Dec 27, 2018

Provide a mechanism for automatic subscription refresh #17

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added autoRefresh option for regular login (non-websocket connections) #34

added autoRefresh option for regular login (non-websocket connections) #34

kprav33n commented Dec 23, 2018

kprav33n commented Dec 23, 2018

codecov bot commented Dec 24, 2018 •

edited

Loading

toravir commented Dec 25, 2018

toravir commented Dec 25, 2018

kprav33n left a comment

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

kprav33n Dec 26, 2018

toravir commented Dec 26, 2018 •

edited

Loading

kprav33n commented Dec 27, 2018

	WS_OPENING = 'Websocket Opening...'
	WS_OPENING = 'Websocket Opening.'

	super(autoRefreshThread, self).__init__()
	super(AutoRefreshThread, self).__init__()

	REFRESH_BEFORE = 60 #approx - this many seconds before expiry, do refresh
	REFRESH_BEFORE = 60 # approx - this many seconds before expiry, do refresh

	CHECK_INTERVAL = 10 #how long to sleep before waking to check
	CHECK_INTERVAL = 10 # how long to sleep before waking to check

	WS_REFRESH_INT = 40 #approx - this many seconds before subscription refresh
	WS_REFRESH_INT = 40 # approx - this many seconds before subscription refresh

	arThread = autoRefreshThread(root)
	arThread = AutoRefreshThread(root)

added autoRefresh option for regular login (non-websocket connections) #34

added autoRefresh option for regular login (non-websocket connections) #34

Conversation

kprav33n commented Dec 23, 2018

kprav33n commented Dec 23, 2018

codecov bot commented Dec 24, 2018 • edited Loading

Codecov Report

toravir commented Dec 25, 2018

toravir commented Dec 25, 2018

kprav33n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toravir commented Dec 26, 2018 • edited Loading

kprav33n commented Dec 27, 2018

codecov bot commented Dec 24, 2018 •

edited

Loading

toravir commented Dec 26, 2018 •

edited

Loading