Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

feature; catalog metadata and caching status published in node events #70

Merged
merged 14 commits into from
Jul 28, 2020

Conversation

kumare3
Copy link
Contributor

@kumare3 kumare3 commented Jun 11, 2020

TL;DR

Caching Information is currently not available in the UI or CLI. This hinders user productivity. This PR sets up the flow of data between propeller and admin. Once the data is published, next step would be to expose it through the API

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Tracking Issue

flyteorg/flyte#138

Follow-up issue

flyteorg/flyte#71

wild-endeavor
wild-endeavor previously approved these changes Jun 11, 2020
Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

any versions you need to bump?

@kumare3
Copy link
Contributor Author

kumare3 commented Jun 12, 2020

any versions you need to bump?

Updated now

@kumare3 kumare3 changed the title Catalog store handling feature; catalog metadata and caching status published in node events Jul 7, 2020
wild-endeavor
wild-endeavor previously approved these changes Jul 7, 2020
Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

no version bump is okay?

also, we're sure we want a different node metadata for every node type right? not a generic nodemetadata message.

Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

merge master or rebase, update setup.py, add comment.

@@ -10,6 +10,7 @@ enum ResourceType {
TASK = 1;
WORKFLOW = 2;
LAUNCH_PLAN = 3;
DATASET = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

One of these things is not like the other. Can we at least add a comment explaining why there is something that looks different here? Also, could we create a GH issue if there isn't one to merge all the proto objects in data catalog https://github.com/lyft/datacatalog/blob/master/protos/idl/service.proto into this repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

@kumare3 kumare3 merged commit fc787dc into master Jul 28, 2020
eapolinario pushed a commit that referenced this pull request Sep 8, 2023
…#70)

* Catalog store handling

* Event proto updated

* generated

* merged

* Dataset Identifier changed

* Better model

* Exposing TaskNodeMetadata with catalog information in the API

* 0.17.38

* Comment

* docs updated
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants