Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update schema and queries with new showDiff variable #624

Conversation

studioswong
Copy link
Contributor

Description

related ticket: https://jira.quantumblack.com/browse/KED-2914

This ticket is to update the schema to reflect the latest design of query and data type to allow the 'showDifference' feature

Development notes

I had mainly made changes to the schema that will be referenced for the BE, as well as added an additional query to showcase the usage of this variable to obtain the showDiff data

QA notes

N/A

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

Legal notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.

  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.

  • I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

Copy link
Member

@tynandebold tynandebold left a comment

Choose a reason for hiding this comment

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

Great stuff!

I don't think we need to create a new query export though, as we should be able to get both the showDiff data and the shared-values data from the same GET_RUN_TRACKING_DATA query.

Comment on lines 42 to 51
/** query for collapsable run details component */
export const GET_RUN_TRACKING_DATA_WITH_DIFF = gql`
query getRunTrackingData($runs: [ID]!, $showDiff: Boolean) {
runTrackingData(runIDs: $runs, showDiff: $showDiff) {
datasetName
datasetType
data
}
}
`;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need this. We should just be able to add the showDiff argument to the GET_RUN_TRACKING_DATA query.

@@ -7,7 +7,7 @@ schema {
type Query {
runsList: [Run]!
runMetadata(runIDs: [ID]!): [Run!]!
runTrackingData(runIDs: [ID]!): [TrackingDataset]!
runTrackingData(runIDs: [ID]!, showDiff: Boolean): [TrackingDataset]! # showDiff as true will return all tracked fields amongst compared runs - default setting is false
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
runTrackingData(runIDs: [ID]!, showDiff: Boolean): [TrackingDataset]! # showDiff as true will return all tracked fields amongst compared runs - default setting is false
runTrackingData(runIDs: [ID]!, showDiff: Boolean = false): [TrackingDataset]!

It would be good to set an explicit default value here for the showDiff argument. That way we can safely leave that argument out of this query when we need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great reminder! Haven't thought of this as I had this as an optional variable in mind but this set up is safer!

@studioswong studioswong merged commit 01bf8ee into experiment-tracking/single-view-journey-1 Nov 11, 2021
@studioswong studioswong deleted the feature/update-schema-for-difference-data branch November 11, 2021 13:59
studioswong added a commit that referenced this pull request Nov 23, 2021
* [KED-2872] Create global toolbar UI (#576)

* adds in the global toolbar component; moves settings and theme buttons to global toolbar

* adds new logo icons for kedro, pipeline, and experiments and adds them to the global sidebar

* adds new test file for global-toolbar; updates primary-toolbar tests

* updates icon styles and order; removes unnecessary tests and initial state values

* removes two unneeded App propTypes

* KED-2875: Set up routing infrastructure and component placeholders for experiment tracking page (#582)

* setup of experiment router route

* setup flowchart Wrapper and refactored old wrapper component

* refadctored sidebar and added experiment route

* Set up runlist sidebar, runsDetails and basic routing

* delete unneeded flowchart wrapper test

* modify route to runsList and refactor to use queryparams for routing

* receive unneccessary change to wrapper scss file

* added comment for experiment details wrapper

* added memoryRouter to test and delete visible prop

* set up useRunIdsFromUrl hook and refactor to use fragments

* change from flowchart to flowChart

* [KED-2882] Create experiment tracking sidebar UI (#587)

* create sidebar runs card; add some simple tests; add necessary colors

* add a collapsable container and necessary tests; small renaming and cleanup

* add comments

* update default accordion header value; remove unneeded margins

* capitalization

* update light bg color on run card; use && instead of ternary; add border-bottom to last run card; remove some placeholder content

* update runs-list-card tests

* [KED-2849] & [KED-2851] Set up GraphQL mock server (#604)

* set up graphql schema

* set up schema and mocks for dev server, along with apollo mock provider and client

* update to runsList component to take in mock data

* updates to mocks

* further updates to mock setup

* update to comments in mocks

* updates to data type of run-list-card

* added fetch library as polyfill for lib-test import

* added new props to runs-list

* set up schema.graphql and updates to schema

* added fetch back in to enable testJS import test to pass on CI

* added fetch and createHttpLink

* [KED-2883] create experiment details UI (#605)

* create a RunMetadata component and style accordingly

* container and single-run styling

* add the show more button to the notes area

* update button font

* starting on the metric details work

* expand accordion component to include another layout and different size titles

* ui driven by common mock-data file

* shared accordions positioned, styled, and collapsing correctly

* better mock data

* scroll metadata and metric together

* move RunMetrics code into own component; style tweaks

* solid refactor of the RunMetrics component

* some renamning; add some comments

* refactor dataset accordions section after new data shape agreed upon

* fix failing RunListCard test

* add a show less button; add more accordion tests

* add tests for RunMetadata and RunDataset components

* add headingClassName to RunsList accordion

* element alignment

* update trackingData shape and refactor RunDataset code accordingly

* [KED-2911] Implement the comparison-view journey (#619)

* update mocks to align with schema from miro; begin runs list selection to display metadata

* toggle comparison view working and updating RunsList; reusing Sidebar and PrimaryToolBar components

* single run selection/deselection working

* comparison run selection/deselection working

* style adjustments; test with dummy graphql server

* fix failing tests

* create a Switch component and wire it up; remove the dummy icon in the PrimaryToolbar

* connect to deployed dummy graphql endpoint

* empty commit to see if pytest works

* pr fixes; create a apollo hooks wrapper function to better transition between query results

* prevent single run deselection in comparison view mode

* more robust test for RunsListCard component

* add the currently designed empty runs screen

Co-authored-by: Rashida Kanchwala <rashida.kanchwala@quantumblack.com>

* update http link to use actal graphql endpoint

* update schema and queries with new showDiff variable (#624)

* update schema and queries with new showDiff variable

* updated schema and queries per PR comments

* updates to queries and tracking details component

* added queries back in

* added changes to component level for graphql data

* further modify naming of value

* KED-2944: integrate the real API with experiment tracking frontend (#630)

* update http link to use actal graphql endpoint

* updates to queries and tracking details component

* added queries back in

* added changes to component level for graphql data

* [KED-2954] Format experiment tracking timestamp (#625)

* create a toHumanReadableTime date util and use in experiment tracking

* fix failing tests

* graphql endpoint back to local viz url

* update formatting for timestamp in toHumanReadableTime function

* simplify toHumanReadableTime logic

* update lock file

Co-authored-by: Tynan DeBold <thdebold@gmail.com>
Co-authored-by: Rashida Kanchwala <rashida.kanchwala@quantumblack.com>
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