Client Library HIL | query calls #617

SahilTikale · 2016-06-20T08:23:16Z

Current PR, includes the basic code structure required to built a client library for HIL.
It also includes client library support for all query calls.

Still pending:
Unit tests for the new code.
Making correspondng cli calls to go via Client library.

includes a unit test suite

-- code for show_xx calls. -- Some code refinement

Conflicts: haas/cli.py

zenhack · 2016-07-08T18:22:42Z

@SahilTikale, you'll need to work in the changes from #618.

Also, could you go over this for style issues? Lots of PEP8/PEP257 violations here. please make sure you've read those documents and understood them, and follow the guidelines. The codebase is full of deviations in general, but let's not make it worse.

Additionally, if you're using vim, I reccomend python-mode, which will warn you about a lot of these issues, plus things like unused imports, and will add other nice python-aware stuff to your editor as well:

https://github.com/klen/python-mode

@gsilvis, @henn, can we prioritize the pep8 testing soonish?

shuwens · 2016-07-09T16:12:45Z

I would point out that Emacs users could adopt py-yapf and YAPF is also interesting to look into.

zenhack · 2016-07-10T01:11:21Z

yapf looks interesting; my experience with gofmt has been excellent, it would be great to have something like that for us. It's says the thing is still fairly immature though, so I'm worried about upgrading versions and then have my editor accidentally change a bunch of formatting without me having actually made any meaningful changes to the file. Even worse would be if two developers were using different versions of the tool, and kept thrashing, making surrious changes to the source...

I gave it a try and it made a few odd choices, e.g.

-        return model.NetworkAttachment.query.filter(
-            model.NetworkAttachment.nic == nic,
-            query,
-        ).count() != 0
+        return model.NetworkAttachment.query.filter(model.NetworkAttachment.nic
+                                                    == nic,
+                                                    query, ).count() != 0

...but nothing I think not worth tolerating to just not spend review time on formatting issues at all.

I'd want to verify we can rely on consistent output from release to release, but given that I love the idea of instituting this.

1. Configure haas.cfg 2. Instantiate the database 3. Populate the database using dummy objects via restful requests 4. Tear down the setup in a clean fashion.

…alls Conflicts: haas/cli.py

henn · 2016-08-09T05:52:27Z

haas/client/node.py

+ q = requests.get(url, headers={"Authorization": "Basic %s" %self.auth})
+ return q.json()
+
+


blank lines

shuwens

I go through the test/client file and it looks ok.

However, when I was trying to deploy it and use it in devhaas, I hit this error:

(.venv)[shwsun@hack-n-hil: haas (HILclientlib-querycalls)]$ ./devhaas 
+ haas switch_register dell-247 powerconnect55xx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTP connection (1): 127.0.0.1
DEBUG:requests.packages.urllib3.connectionpool:http://127.0.0.1:5050 "PUT /switch/dell-247 HTTP/1.1" 200 0

+ haas switch_register cisco-1 nexus xxxxxxxxxxxxxxxxxxxxxxxxxxx
DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTP connection (1): 127.0.0.1
DEBUG:requests.packages.urllib3.connectionpool:http://127.0.0.1:5050 "PUT /switch/cisco-1 HTTP/1.1" 200 0

+ haas node_register sun-20 ipmi xxxxxxxxxxxxxxxxxxxxxxxxxxx
DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTP connection (1): 127.0.0.1
DEBUG:requests.packages.urllib3.connectionpool:http://127.0.0.1:5050 "PUT /node/sun-20 HTTP/1.1" 200 0

+ haas port_register dell-247 gi1/0/21
DEBUG:requests.packages.urllib3.connectionpool:Starting new HTTP connection (1): 127.0.0.1
Error: HTTPConnectionPool(host='127.0.0.1', port=5000): Max retries exceeded with url: /switch/dell-247/port/gi1/0/21 (Caused by NewConnectionError('<requests.packages.urllib3.connection.HTTPConnection object at 0x3cee990>: Failed to establish a new connection: [Errno 111] Connection refused',))

I also tried the currect haas and it is fine

shuwens · 2017-04-14T17:54:55Z

tests/unit/client.py

+def make_config():
+ """ This function creates haas.cfg with desired options
+ and writes to a temporary directory.
+ It returns a tuple where (tmpdir, cwd) = ('location of haas.cfg', 'pwdd')


Looks like your configuration has some problems.
Is the server running on port 5000 or port 5050

DEBUG:requests.packages.urllib3.connectionpool:http://127.0.0.1:5050
or
Error: HTTPConnectionPool(host='127.0.0.1', port=5000): Max retries exceeded with url...

Create an init_client file with correct values for following parameters.

export HAAS_ENDPOINT=http://<ip address + port no. where haas server is running> export HAAS_USERNAME=<authorized username> export HAAS_PASSWORD=<password for that username>

source this init_client file in your environment as we normally do.

thanks for catching the typo, next commit fixes it.

naved001 · 2017-04-14T19:47:34Z

@shwsun you should edit your comment to hide login details in the error log. this page is public.

shuwens · 2017-04-14T20:06:52Z

@naved001 good catch, thank you!

zenhack · 2017-04-18T04:12:20Z

@naved001, @shwsun, I didn't catch what it was that got leaked, and I'm sure this goes without saying, but if there were credentials in there, change them.

… to the top of test file. corrects a typo in comment.

SahilTikale · 2017-04-18T11:21:14Z

@zenhack, I believe I have addressed all your comments, let me know if anything else is needed.

shuwens · 2017-04-18T13:39:37Z

@naved001, @shwsun, I didn't catch what it was that got leaked, and I'm sure this goes without saying, but if there were credentials in there, change them.

@zenhack The thing that naved pointed out were some ip addr/passwd that I included previously in my comment, those sensitive info were already gotten rid of.

zenhack · 2017-04-19T04:51:42Z

tests/unit/client.py

 time.sleep(1)
 else:
- break
+ return que
+ return timeout


I would just raise an exception here (of a newly-declared type, which denotes a timeout), and get rid of the if/else checks elsewhere.

zenhack · 2017-04-20T20:27:27Z

@SahilTikale, travis is failing. Given that it's just postgres I'm wondering whether 10 seconds is enough to be reliable, but I'm not totally sure what's going on.

Other than that I'm happy; get the tests passing reliably again and I'll approve.

SahilTikale · 2017-04-20T22:35:25Z

@zenhack passes all the tests.
Time out is not a problem. sometimes it is not able to spin up the server on the designated port and that causes failures.

SahilTikale · 2017-04-21T01:25:37Z

@shwsun only your approval is pending.

Let me know if you need me to address any issues.

shuwens · 2017-04-21T01:46:39Z

@SahilTikale I see, could you be around sometime tmr? I still have some questions on deployment

naved001 · 2017-04-21T21:42:14Z

@SahilTikale The tests are failing again. Probably due to the race condition thing you mentioned, it might even pass if we re-run this.

socket.error: [Errno 98] Address already in use

But I think we should reliably fix that test here, otherwise we would have a tough time with CI failing on future PRs.

zenhack · 2017-04-21T22:20:57Z

@SahilTikale, I agree with @naved001; the tests need to pass reliably.

SahilTikale · 2017-04-22T01:16:53Z

@naved001 the race condition was a different problem. There is a function that solves it reliably.
This is a different issue of not being able to find the port no to spin up the server and it is not that frequent.

@zenhack there is nothing to worry.

SahilTikale · 2017-04-22T01:18:08Z

@shwsun I have addressed the issue we were discussing today.
Let me know if anything else is needed.

SahilTikale · 2017-04-24T05:07:06Z

@shwsun I would like to get this PR merged if there is nothing else that you would want me to change.

Let me know if you need anything else.

shuwens · 2017-04-24T05:35:07Z

I dont think I have any other more concerns. +1

SahilTikale · 2017-04-24T10:59:20Z

@zenhack would you merge it (Yeahh, finally !!) or should I do it.

SahilTikale added 4 commits June 6, 2016 08:22

client library support for HIL query calls.

e75786c

includes a unit test suite

Adds files containing common code to build client library.

61bb87d

Merge branch 'master' into HILclientlib-querycalls

3d8ca6d

Adds client library support for query calls

ee5aa40

SahilTikale added the do not merge yet label Jun 20, 2016

SahilTikale added 6 commits July 3, 2016 00:21

Adds client support to CLI.

529b37c

Adds

e5cdad3

-- code for show_xx calls. -- Some code refinement

Adds unit tests and some code refactor.

060a55e

Addresses unit tests of client Base class

5237d61

Merge branch 'master' into HILclientlib-querycalls

dc4ffea

Conflicts: haas/cli.py

Supports new call list_nodes in place of list_free_nodes.

cae9dd4

SahilTikale mentioned this pull request Jul 11, 2016

Introduce python client library for HaaS #498

Closed

SahilTikale added 6 commits July 25, 2016 13:24

adds __init__ file

06b197a

making unit tests to work

f9eb580

Includes a clean way to

2e63285

1. Configure haas.cfg 2. Instantiate the database 3. Populate the database using dummy objects via restful requests 4. Tear down the setup in a clean fashion.

unit tests pass.

f36ac1f

Merge remote-tracking branch 'origin/master' into HILclientlib-queryc…

ddf43d4

…alls Conflicts: haas/cli.py

Fixes unit tests

595c30d

SahilTikale self-assigned this Aug 8, 2016

increasing delay to fix tests

b0106ac

henn reviewed Aug 9, 2016
View reviewed changes

refactors . Addresses other suggestions by reviewers.

1cd832e

shuwens reviewed Apr 14, 2017

View reviewed changes

Improves tests, provides explicit message for time out. Moves imports…

22800ab

… to the top of test file. corrects a typo in comment.

zenhack reviewed Apr 19, 2017

View reviewed changes

minorly improves

157e518

zenhack approved these changes Apr 21, 2017

View reviewed changes

Catches error of not setting HAAS_ENDPOINT.

922437f

shuwens approved these changes Apr 24, 2017

View reviewed changes

SahilTikale added ready to merge and removed waiting on reviewer labels Apr 24, 2017

zenhack merged commit 384623a into CCI-MOC:master Apr 24, 2017

shuwens mentioned this pull request Apr 24, 2017

Merge of CLI lib just breaks the CI test. #759

Closed

henn mentioned this pull request Apr 27, 2017

URGENT PATCH: Fixes tests of client library #762

Merged

naved001 mentioned this pull request May 16, 2018

Make client tests faster #1014

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Client Library HIL | query calls #617

Client Library HIL | query calls #617

SahilTikale commented Jun 20, 2016

zenhack commented Jul 8, 2016

shuwens commented Jul 9, 2016

zenhack commented Jul 10, 2016

henn Aug 9, 2016

SahilTikale Feb 1, 2017

shuwens left a comment •

edited

Loading

shuwens Apr 14, 2017

SahilTikale Apr 18, 2017

SahilTikale Apr 18, 2017

SahilTikale Apr 18, 2017

naved001 commented Apr 14, 2017

shuwens commented Apr 14, 2017

zenhack commented Apr 18, 2017

SahilTikale commented Apr 18, 2017

shuwens commented Apr 18, 2017

zenhack Apr 19, 2017

zenhack commented Apr 20, 2017

SahilTikale commented Apr 20, 2017

SahilTikale commented Apr 21, 2017

shuwens commented Apr 21, 2017

naved001 commented Apr 21, 2017

zenhack commented Apr 21, 2017

SahilTikale commented Apr 22, 2017 •

edited

Loading

SahilTikale commented Apr 22, 2017

SahilTikale commented Apr 24, 2017

shuwens commented Apr 24, 2017

SahilTikale commented Apr 24, 2017 •

edited

Loading

		q = requests.get(url, headers={"Authorization": "Basic %s" %self.auth})
		return q.json()

Client Library HIL | query calls #617

Client Library HIL | query calls #617

Conversation

SahilTikale commented Jun 20, 2016

zenhack commented Jul 8, 2016

shuwens commented Jul 9, 2016

zenhack commented Jul 10, 2016

henn Aug 9, 2016

Choose a reason for hiding this comment

SahilTikale Feb 1, 2017

Choose a reason for hiding this comment

shuwens left a comment • edited Loading

Choose a reason for hiding this comment

shuwens Apr 14, 2017

Choose a reason for hiding this comment

SahilTikale Apr 18, 2017

Choose a reason for hiding this comment

SahilTikale Apr 18, 2017

Choose a reason for hiding this comment

SahilTikale Apr 18, 2017

Choose a reason for hiding this comment

naved001 commented Apr 14, 2017

shuwens commented Apr 14, 2017

zenhack commented Apr 18, 2017

SahilTikale commented Apr 18, 2017

shuwens commented Apr 18, 2017

zenhack Apr 19, 2017

Choose a reason for hiding this comment

zenhack commented Apr 20, 2017

SahilTikale commented Apr 20, 2017

SahilTikale commented Apr 21, 2017

shuwens commented Apr 21, 2017

naved001 commented Apr 21, 2017

zenhack commented Apr 21, 2017

SahilTikale commented Apr 22, 2017 • edited Loading

SahilTikale commented Apr 22, 2017

SahilTikale commented Apr 24, 2017

shuwens commented Apr 24, 2017

SahilTikale commented Apr 24, 2017 • edited Loading

shuwens left a comment •

edited

Loading

SahilTikale commented Apr 22, 2017 •

edited

Loading

SahilTikale commented Apr 24, 2017 •

edited

Loading