andreaspeters commented on a change in pull request #383:
URL: https://github.com/apache/mesos/pull/383#discussion_r662804747
##########
File path: src/python/cli_new/lib/cli/http.py
##########
@@ -19,70 +19,70 @@
"""
import json
-import urllib.request
-import urllib.error
-import urllib.parse
-import time
+from urllib.parse import urlencode
+import urllib3
import cli
from cli.exceptions import CLIException
+# Disable all SSL warnings. These are not necessary, as the user has
+# the option to disable SSL verification.
+urllib3.disable_warnings()
-def read_endpoint(addr, endpoint, query=None):
+def read_endpoint(addr, endpoint, config, query=None):
"""
Read the specified endpoint and return the results.
"""
+
try:
addr = cli.util.sanitize_address(addr)
except Exception as exception:
raise CLIException("Unable to sanitize address '{addr}': {error}"
.format(addr=addr, error=str(exception)))
-
try:
url = "{addr}/{endpoint}".format(addr=addr, endpoint=endpoint)
if query is not None:
- url += "?{query}".format(query=urllib.parse.urlencode(query))
- http_response = urllib.request.urlopen(url).read().decode("utf-8")
+ url += "?{query}".format(query=urlencode(query))
+ if config.principal() is not None and config.secret() is not None:
+ headers = urllib3.make_headers(
+ basic_auth=config.principal() + ":" + config.secret()
+ )
+ else:
+ headers = None
+ http = urllib3.PoolManager()
+ http_response = http.request(
+ 'GET',
+ url,
+ headers=headers,
+ timeout=config.agent_timeout()
+ )
+ return http_response.data.decode('utf-8')
+
except Exception as exception:
raise CLIException("Unable to open url '{url}': {error}"
.format(url=url, error=str(exception)))
- return http_response
-
-def get_json(addr, endpoint, condition=None, timeout=5, query=None):
+def get_json(addr, endpoint, config, condition=None, query=None):
"""
Return the contents of the 'endpoint' at 'addr' as JSON data
subject to the condition specified in 'condition'. If we are
- unable to read the data or unable to meet the condition within
- 'timeout' seconds we throw an error.
+ unable to read the data we throw an error.
"""
- start_time = time.time()
- while True:
- data = None
+ data = read_endpoint(addr, endpoint, config, query)
- try:
- data = read_endpoint(addr, endpoint, query)
- except Exception as exception:
- pass
-
- if data:
- try:
- data = json.loads(data)
- except Exception as exception:
- raise CLIException("Could not load JSON from '{data}': {error}"
- .format(data=data, error=str(exception)))
-
- if not condition:
- return data
+ try:
+ data = json.loads(data)
+ except Exception as exception:
+ raise CLIException("Could not load JSON from '{data}': {error}"
+ .format(data=data, error=str(exception)))
- if condition(data):
- return data
+ if not condition:
+ return data
- if time.time() - start_time > timeout:
- raise CLIException("Failed to get data within {seconds} seconds"
- .format(seconds=str(timeout)))
+ if condition(data):
Review comment:
Actually I'm not 100% sure what the idea behind `condition` was. But
yes, `data` have ever be to returned. But if `condition` is set, then through
that function. I will create (for myself) a issue to investigate `condition` to
see if it's needed or if we can remove it. Then we can merge this PR. Is this
procedure ok for everyone? :-)
##########
File path: src/python/cli_new/lib/cli/http.py
##########
@@ -19,70 +19,70 @@
"""
import json
-import urllib.request
-import urllib.error
-import urllib.parse
-import time
+from urllib.parse import urlencode
+import urllib3
import cli
from cli.exceptions import CLIException
+# Disable all SSL warnings. These are not necessary, as the user has
+# the option to disable SSL verification.
+urllib3.disable_warnings()
-def read_endpoint(addr, endpoint, query=None):
+def read_endpoint(addr, endpoint, config, query=None):
"""
Read the specified endpoint and return the results.
"""
+
try:
addr = cli.util.sanitize_address(addr)
except Exception as exception:
raise CLIException("Unable to sanitize address '{addr}': {error}"
.format(addr=addr, error=str(exception)))
-
try:
url = "{addr}/{endpoint}".format(addr=addr, endpoint=endpoint)
if query is not None:
- url += "?{query}".format(query=urllib.parse.urlencode(query))
- http_response = urllib.request.urlopen(url).read().decode("utf-8")
+ url += "?{query}".format(query=urlencode(query))
+ if config.principal() is not None and config.secret() is not None:
+ headers = urllib3.make_headers(
+ basic_auth=config.principal() + ":" + config.secret()
+ )
+ else:
+ headers = None
+ http = urllib3.PoolManager()
+ http_response = http.request(
+ 'GET',
+ url,
+ headers=headers,
+ timeout=config.agent_timeout()
+ )
+ return http_response.data.decode('utf-8')
+
except Exception as exception:
raise CLIException("Unable to open url '{url}': {error}"
.format(url=url, error=str(exception)))
- return http_response
-
-def get_json(addr, endpoint, condition=None, timeout=5, query=None):
+def get_json(addr, endpoint, config, condition=None, query=None):
"""
Return the contents of the 'endpoint' at 'addr' as JSON data
subject to the condition specified in 'condition'. If we are
- unable to read the data or unable to meet the condition within
- 'timeout' seconds we throw an error.
+ unable to read the data we throw an error.
"""
- start_time = time.time()
- while True:
- data = None
+ data = read_endpoint(addr, endpoint, config, query)
- try:
- data = read_endpoint(addr, endpoint, query)
- except Exception as exception:
- pass
-
- if data:
- try:
- data = json.loads(data)
- except Exception as exception:
- raise CLIException("Could not load JSON from '{data}': {error}"
- .format(data=data, error=str(exception)))
-
- if not condition:
- return data
+ try:
+ data = json.loads(data)
+ except Exception as exception:
+ raise CLIException("Could not load JSON from '{data}': {error}"
+ .format(data=data, error=str(exception)))
- if condition(data):
- return data
+ if not condition:
+ return data
- if time.time() - start_time > timeout:
- raise CLIException("Failed to get data within {seconds} seconds"
- .format(seconds=str(timeout)))
+ if condition(data):
Review comment:
Actually I'm not 100% sure what the idea behind condition was. But yes,
data have ever be to returned. But if condition is set, then through that
function. I will create (for myself) a issue to investigate condition to see if
it's needed or if we can remove it. Then we can merge this PR. Is this
procedure ok for everyone? :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
