Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Support APS API - #2

Open
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master
Open

Support APS API#2
ikrasner wants to merge 12 commits into
cloudblue:masterfrom
ikrasner:master

Conversation

@ikrasner

@ikrasnerikrasner commented May 29, 2017

Copy link
Copy Markdown

Add basic support of APS controller interface http://doc.apsstandard.org/7.1/api/

@hayorovhayorov changed the title Add aps api Support APS APIMay 29, 2017
Comment threadapsapi/__init__.py Outdated
try:
import json
except ImportError:
import simplejson as json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We realy need support python < 2.6?

Comment threadapsapi/__init__.py Outdated

return content

def GET(self, path, headers=None, data=None, cert=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use upper in method name

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.getConnection, req, context=self._context)
return self.do_open(self.getConnection, req)

def getConnection(self, host, context=None, timeout=300):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use camelcase in method name

Comment threadapsapi/__init__.py Outdated
class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

def __init__(self, key, cert):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between method declaration and logic

Comment threadapsapi/__init__.py Outdated


class HTTPSClientAuthHandler(urllib2.HTTPSHandler):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't use blank lines between class declaration and methods

Comment threadapsapi/__init__.py Outdated

class API:

verbose = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we use it?

Comment threadapsapi/__init__.py Outdated
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert)


class API:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit from object

Comment threadapsapi/__init__.py Outdated

def getConnection(self, host, context=None, timeout=300):
if context is not None:
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't copy code
If you have context create some like extra and use it

extra = {}
if context:
extra['context'] = context
httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, **extra)

Comment threadapsapi/__init__.py Outdated
elif (isinstance(data, (dict, list))):
data = json.dumps(data)

url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use urljoin

Comment threadapsapi/__init__.py Outdated

url = self.url + path

if not(headers):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if not headers:

Comment threadapsapi/__init__.py Outdated
# ('something' in error.aps.message) we convert this exception
# to JSON directly here.
error.aps = API.APSExcStruct()
if len(contents):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if contents, not need len

Comment threadapsapi/__init__.py Outdated
@@ -0,0 +1,122 @@
#!/usr/bin/python

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove it

Comment threadapsapi/__init__.py Outdated
context = None
if _PYTHON_2_7_9_COMPAT:
context = ssl._create_unverified_context()
urllib2.HTTPSHandler.__init__(self, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use super
add blank line before/after super

Comment threadapsapi/__init__.py Outdated
return self.do_open(self.get_connection, req, context=self._context)

def get_connection(self, host, context=None):
return httplib.HTTPSConnection(host, key_file=self.key, cert_file=self.cert, context=context)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why we can't use self._context as in https_open method?

Comment threadapsapi/__init__.py Outdated

class API(object):

class APSExcStruct:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move outside

Comment threadapsapi/__init__.py Outdated
def call(self, verb, path, headers=None, data=None, cert=None, rheaders=None):
data = json.dumps(data)

# url = self.url + path

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop comented code

Comment threadapsapi/__init__.py Outdated
opener = urllib2.build_opener(HTTPSClientAuthHandler(cert, cert))
resp = opener.open(req)
else:
if _PYTHON_2_7_9_COMPAT:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use logic like in HTTPSClientAuthHandler.__init__ with context as None by default

Comment threadapsapi/__init__.py Outdated
resp = urllib2.urlopen(req, context=context)
else:
resp = urllib2.urlopen(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop blank line

Comment threadapsapi/__init__.py Outdated
# "type": "APS::Hosting::Exception",
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's need here? Maybe move it in APSExcStruct?

Comment threadapsapi/__init__.py Outdated
return self.call('DELETE', path, headers, data, cert)

#
# Usage Example:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move example from code in README

Comment threadapsapi/__init__.py Outdated


class API(object):
def __init__(self, url, use_unverified_context):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set default for use_unverified_context

Comment threadapsapi/__init__.py Outdated
# "message": "Limit for resource ..."
# }
# In order to allow simple processing of this exception like
# ('something' in error.aps.message) we convert this exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove APSExcStruct you need remove this comment too

Comment threadapsapi/__init__.py
from urlparse import urljoin


class HTTPSClientAuthHandler(urllib2.HTTPSHandler, object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use only urllib2.HTTPSHandler

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

HTTPSHandler is old-style class with out object I cannot use super() in init()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Comment threadapsapi/__init__.py Outdated
self.use_unverified_context = use_unverified_context

def call(self, verb, path, headers=None, data=None, cert=None):
data = json.dumps(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if data is None then the resulting json would be null which is a weird body for REST. Use smth like

ifnotdata:
data= {}

Comment threadapsapi/__init__.py Outdated

url = urljoin(self.url, path)
if not headers:
headers = dict()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

headers= {}

is a more common practice, with better performance

Comment threadapsapi/__init__.py
data = json.dumps(data)
if not data:
data = {}
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This else is a typo, isn't it? json.dumps should be called unconditionally.

@hayorovhayorov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ikrasner Looks like you implemented basic and generic REST-like API wrapper over urllib2 without significant APS specificity like:

  • Native collections in binding code eg. resources, types, application
  • RQL filtering support
  • Pagination
  • Native APS headers

Looks "so-so", Ok for first version and just usage generalisation.
But its possible to use Slumber or any other More advanced REST interface binding for resolve this simple task.

Comment threadREADME.md
---------------------------------------
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

url = 'h -> url='h

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

safilter what is it? I can't find in README.md any information about it.

Comment threadREADME.md
```{.sourceCode .python}
import apsapi
aapi = apsapi.API(url = 'https://hostname:6308')
resp = aapi.GET( '/aps/2/resources/' + safilter, {'APS-Token': token} )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aapi.GET( '/aps/2/resources/' )
Extra spaces in doc code, please check and fix

Comment threadapsapi/__init__.py
def __init__(self, url, use_unverified_context=False):
"""
Takes something like the following argument as input:
url = 'https://a.bvt.aps.sw.ru:6308'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use depersonalized example instead eg. hostname or example.com

Comment threadsetup.py
packages=['osaapi', 'apsapi'],
url='https://aps.odin.com',
license='Apache License',
description='A python client for the Odin Service Automation (OSA) and billing APIs.',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A python binding for Odin Automation and APS controller https://doc.apsstandard.org/7.1/api/rest/

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ikrasner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ikrasner@CLAassistant@spukst3r@ToxicWar@hayorov