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

bug(cli): handle case insensitive entity types for cli delete #4492

Merged

Conversation

RyanHolstien
Copy link
Collaborator

@RyanHolstien RyanHolstien commented Mar 24, 2022

Checklist

or entity_type == "dataflow"
or entity_type == "datajob"
or entity_type == "container"
and entity_type_lower == "dataset"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there anywhere else we do these types of entity type comparisons in the CLI?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are a few others, but they look to be already using .lower() or are in tests

@github-actions
Copy link

Unit Test Results (build & test)

  92 files  +  1    92 suites  +1   14m 43s ⏱️ + 8m 1s
674 tests +77  614 ✔️ +69  59 💤 +7  1 +1 

For more details on these failures, see this check.

Results for commit 41cc59c. ± Comparison against base commit c9b40ec.

@github-actions
Copy link

Unit Test Results (metadata ingestion)

       5 files         5 suites   47m 35s ⏱️
   371 tests    371 ✔️   0 💤 0
1 702 runs  1 664 ✔️ 38 💤 0

Results for commit 41cc59c.

Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

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

LGTM!

@shirshanka shirshanka merged commit d311067 into datahub-project:master Mar 31, 2022
@RyanHolstien RyanHolstien deleted the bugfix/handleLowerCaseEntity branch March 31, 2022 22:30
maggiehays pushed a commit to maggiehays/datahub that referenced this pull request Aug 1, 2022
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.

3 participants