GMIM-353: Add utility method to fetch widget data... #79
Conversation
…ue event_timeline.
|
Is there any context on these changes? I'm unable to access the GMI jira project to look at the ticket |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #79 +/- ##
==========================================
- Coverage 22.54% 22.50% -0.05%
==========================================
Files 11 11
Lines 1433 1480 +47
Branches 342 351 +9
==========================================
+ Hits 323 333 +10
- Misses 1110 1147 +37 ☔ View full report in Codecov by Sentry. |
|
@ekauzlarich can you take a look and re-review? |
ekauzlarich
changed the title
| raise ValueError("Error - {}".format(response.text)) | ||
| data = response.json()["panels"] | ||
| except Exception as e: | ||
| raise ValueError(e) |
There was a problem hiding this comment.
is there a reason for covering up the original exception? at least raise ... from e would be good to keep the stack context
| @@ -52,6 +61,10 @@ def create_share_link( | |||
| url_params = {} | |||
| url_params["state_hash"] = str(uuid.uuid4())[:8] | |||
| url_params["context"] = "/analysis/datavis" | |||
| try: | |||
| url_params["in_use_workspace"] = self.session.headers["X-Sm-Workspace-Id"] | |||
There was a problem hiding this comment.
use the resource constant: X_SM_WORKSPACE_ID instead of magic string
| url_params["state_hash"] = str(uuid.uuid4())[:8] | ||
| url_params["context"] = context | ||
| try: | ||
| url_params["in_use_workspace"] = self.session.headers["X-Sm-Workspace-Id"] |
There was a problem hiding this comment.
use the resource constant: X_SM_WORKSPACE_ID instead of magic string
| try: | ||
| url_params["in_use_workspace"] = self.session.headers["X-Sm-Workspace-Id"] | ||
| except: | ||
| pass |
There was a problem hiding this comment.
why is this caught and passed? what would be the reason for this header missing and why can it be ignored?
| {"field": {"name": "offset_endtime", "type": "datetime"}} | ||
| ] | ||
| if are_line_params_available: | ||
| pass |
There was a problem hiding this comment.
can be raised an indent level and changed to if model == "line" and not are_line_params_available: and remove the pass/else branch for readability
| def create_widget_share_link(self, context, **kwargs): | ||
| url = "{}{}".format(self.base_url, ENDPOINTS["DataViz"]["share_link"]) | ||
| url_params = {} | ||
| url_params["state_hash"] = str(uuid.uuid4())[:8] |
There was a problem hiding this comment.
what is the use of this param? this isn't making a hash of anything currently
| try: | ||
| url_params["in_use_workspace"] = self.session.headers["X-Sm-Workspace-Id"] | ||
| except: | ||
| pass |
There was a problem hiding this comment.
same except: pass comment from above here
| html_content = self.session.get(url).text | ||
| soup = BeautifulSoup(html_content, "html.parser") | ||
| table = soup.find("table") | ||
| rows = table.find("tbody").find_all("tr") |
There was a problem hiding this comment.
can you simplify via something like rows = soup.select("table tbody tr") ?
There was a problem hiding this comment.
also I'm unfamiliar with what is callable from this SDK or not, is /dev/udf/notebooks available instead of parsing the HTML table? this is prone to breaking if the page is ever updated
| async_task_id: str = results.json().get("response").get("task_id") | ||
|
|
||
| results = self.session.get(url + "/" + async_task_id).json() | ||
| time.sleep(10) |
There was a problem hiding this comment.
instead of sleeping manually 10s twice, this should loop until a specified timeout for task completion
|
|
||
| if "machine__source" not in kwargs and "machine__source__in" not in kwargs: | ||
| log.warn("Machine source not specified.") | ||
| return [] |
There was a problem hiding this comment.
where is this function called? also should this return a request error of some sort since it seems like these params are required for it to do anything?
There was a problem hiding this comment.
will take a look at these. Thanks for feedback
No description provided.