Skip to content

[Eng-585] kpi support - #26

Merged
mgoldmansight merged 9 commits into
masterfrom
ENG-585_KPI_support
Mar 8, 2023
Merged

[Eng-585] kpi support#26
mgoldmansight merged 9 commits into
masterfrom
ENG-585_KPI_support

Conversation

@mgoldmansight

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread smsdk/config/api_endpoints.json Outdated
"task": "/v1/datavis/task/async"
},
"KPI": {
"availible_kpis": "/v1/selector/datavis/kpi/y_axis"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is the API to get the names of KPIs given a list of machineTypes. Let's use the assets API instead to get the full list of KPIs. In addition to not requiring a payload, this will give us all the kpi info (name, formula, dependencies...), not just the name.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

assets API is GET /v1/selector/assets/

Comment thread smsdk/client.py
f"title_prefix :: {stat.get('display', {}).get('title_prefix', '')}")
return fields

def get_kpis(self, **kwargs):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please add docstrings for the public methods, so that users can discover usage using help()

@srblum srblum left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FYI, using
pip3 install git+https://github.com/sightmachine/sightmachine-sdk.git@ENG-585_KPI_support

I was able to test the new APIs, and get the error

>>> cli.get_kpis()
Traceback (most recent call last):
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/smsdk/ma_session.py", line 185, in _get_records_v1
    raise ValueError("Error - {}".format(response.text))
ValueError: Error - {"description":"Some of the submitted information was invalid","details":{"fields":{"asset_selection":{"message":"Missing value","value":null}}},"error":"form_invalid"}

(which is because you're using the wrong API--mentioned elsewhere.)

I still haven't been able to use it successfully just importing from the smsdk directory directly--or rather, I can instantiate a Client and login just fine, but when I actually try to fetch some data, I get:

>>> cli.get_machine_type_names()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/smsdk/client_v0.py", line 701, in get_machine_type_names
    machine_types = self.get_data('machine_type', 'get_machine_types', normalize=True, **query_params)
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/smsdk/client_v0.py", line 205, in get_data
    cls = smsdkentities.get(ename)(self.session, base_url)
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/smsdk/register.py", line 100, in get
    initialize()
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/smsdk/register.py", line 49, in initialize
    importlib.import_module(pkg_info.name)
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 843, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/smsdk/smsdk_entities/cycle/cycle.py", line 21, in <module>
    class Cycle(SmsdkEntities, MaSession):
  File "/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/site-packages/smsdk/register.py", line 81, in decorator
    raise ValueError(f"{key} is already used for {existing!r}")
ValueError: ('cycle', 1) is already used for <class 'smsdk_entities.cycle.cycle.Cycle'>

This is true even on master--there's probably something stupid I'm doing wrong. I will keep trying later, but if you can figure out that workflow, it will be much faster.

@srblum srblum left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks pretty good. Just needs some proofreading and minor tweaks.

Also, I haven't tested it, but it the user passes an invalid time_selection it's important that they be able to see the validation error returned from the API. Can they?

Comment thread docs/entities/kpis.md Outdated
KPIs are user defined calculated fields in the Sight Machine software.

## Functions
The SDK has three functions related to KPIs. The first returns a list of all availible KPis. The second of which allows a user to see which KPIs are availible for a particular asset. The thrid makes use of our Data Visulation api which allows a user to see these KPIs over a timeframe.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typos

thrid -> third
Visulation -> Visualization

Comment thread docs/entities/kpis.md Outdated
The SDK has three functions related to KPIs. The first returns a list of all availible KPis. The second of which allows a user to see which KPIs are availible for a particular asset. The thrid makes use of our Data Visulation api which allows a user to see these KPIs over a timeframe.

### Get KPIs
This is the first KPI function allowing you to see which all KPIs. In order to call this function you must first have a logged in client see the [quick start guide](/README.md) for more information on logging in. Once you have a logged in client you can call the function as follows:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is the first KPI function allowing you to see which all KPIs

sentence doesn't quite make sense

The value to compare with the operation.

## db_mode
This is optional. It will defualt to 'sql' and usually should be but we have a 'monogo' mode as well. You will likely ever need to tset this. No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typos
defualt -> default
monogo -> mongo

and also

You will likely ever need to tset this.

Comment thread smsdk/client.py Outdated
)
return kpis(self.session, base_url).get_kpis_for_asset(**kwargs)

def get_kpi_data_viz(self, machine_source=None, kpis=None, i_vars=None, time_selection=None, **kwargs):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's have this take a machines array, not just a single machine, since that's more flexible, and the API supports it.

@srblum srblum left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, although you missed one of the comments:

Typos
defualt -> default
monogo -> mongo

and also

You will likely ever need to tset this.

@mgoldmansight
mgoldmansight merged commit bacdba5 into master Mar 8, 2023
Sign up for free to 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.

2 participants