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

feat: add data elements to demo #1232

Merged
merged 2 commits into from
Apr 24, 2023
Merged

feat: add data elements to demo #1232

merged 2 commits into from
Apr 24, 2023

Conversation

czgu
Copy link
Collaborator

@czgu czgu commented Apr 22, 2023

  • also fixed some issues with string data element
  • added tags and data element in demo
  • hide data element search if it is not available

@czgu czgu requested a review from jczhong84 April 22, 2023 02:46
querybook/server/logic/data_element.py Outdated Show resolved Hide resolved
Comment on lines +135 to +143
data_element = None
if data_element_tuple:
if type(data_element_tuple) is str:
data_element = get_data_element_by_name(data_element_tuple, session=session)
elif data_element_tuple.name and data_element_tuple.type:
data_element = create_or_update_data_element(
metastore_id, data_element_tuple, session=session
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

does it need to check the data_element is None again here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i added a check on line 148

Comment on lines +353 to +355
queryMetastores.find((metastore) => metastore.id === metastoreId);
const queryMetastoreHasDataElements =
!!queryMetastore?.flags?.has_data_element;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: queryMetastores.some((metastore) => metastore.id === metastoreId && !!metastore.flags?.has_data_element)

Copy link
Collaborator Author

@czgu czgu Apr 24, 2023

Choose a reason for hiding this comment

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

i think its better to find the queryMetastore first and then check if it has data elements. Using some make the logic harder to understand and less extensible

@czgu czgu merged commit 8b8fcc9 into pinterest:master Apr 24, 2023
4 checks passed
@czgu czgu deleted the dataelement branch April 24, 2023 17:41
aidenprice pushed a commit to arrowtail-precision/querybook that referenced this pull request Jan 3, 2024
* feat: add data elements to demo

* resolved comments
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.

None yet

2 participants