microsoft / mwt-ds

Umbrella repository for projects related to the MWT Decision Service
187 stars 77 forks source link

Add dashboard mpi #133

Closed cheng-tan closed 5 years ago

msftclas commented 5 years ago

CLA assistant check
All CLA requirements met.

marco-rossi29 commented 5 years ago

I see that dashboard.py contains many functions from ds_parse.py. There is an issue of code duplication. In particular if our dsjson format changes in the future, ideally we should have only one place to change.

To solve this issue, can you import ds_parse and avoid duplication?

marco-rossi29 commented 5 years ago

If importing ds_parse is feasible. Could we also import dashboard_utils.py to basically remove the need for dashboard.py?

cheng-tan commented 5 years ago

ds_parse is used by dashboard_util, no need to import anymore.

ataymano commented 5 years ago

SInce we are going to use it more actively very soon, can we check it in and fix remaining issues afterwards in separate prs (right now no one except @cheng-tan can work on this code)?

marco-rossi29 commented 5 years ago

@ataymano There is a trivial conflict that must be resolved before merging. If you/Cheng can fix it, I'll merge

cheng-tan commented 5 years ago

Resubmitting a PR for dashboard mpi