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

[WIP] Feature/buisness type database changes #15154

Conversation

cccs-RyanS
Copy link
Contributor

@cccs-RyanS cccs-RyanS commented Jun 14, 2021

SUMMARY

Our users are leveraging Superset to explore and visualize network security logs. These logs often contain IP, port, hostname, and/or protocol fields, among others. While such fields might be stored as simple INTEGERS or VARCHARS in the backend database, these fields also have a “business” type which, if leveraged, can provide additional context and improve the user experience.

The following code changes are the first steps towards adding these types currently they are simply added text fields

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@cccs-RyanS cccs-RyanS requested a review from a team as a code owner June 14, 2021 18:59
@cccs-RyanS cccs-RyanS marked this pull request as draft June 14, 2021 19:00
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 6, 2021
@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@betodealmeida betodealmeida self-requested a review July 12, 2021 15:01
@@ -533,6 +533,7 @@ class BaseColumn(AuditMixinNullable, ImportExportMixin):
verbose_name = Column(String(1024))
is_active = Column(Boolean, default=True)
type = Column(String(32))
business_type = Column(String(255))
Copy link
Member

Choose a reason for hiding this comment

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

Hi, @cccs-RyanS... do you want to give more context about this PR before putting work into it? I'm worried that this will need a DB migration, and this columns seems very specific.

Copy link
Contributor Author

@cccs-RyanS cccs-RyanS Jul 15, 2021

Choose a reason for hiding this comment

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

Hey so my org had discussed this with Ville Brofeldt and I was waiting on some feedback (which is why its still a draft with very little context, my apologies). The idea was to have business types on top of the db types to have added flexibility. Let me speak to my coworker and I can get you a bunch of extra context and I will add more context to the PR. Thanks for taking a look!

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

15 similar comments
@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2021

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2021

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2021

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2021

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2021

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2021

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

6 similar comments
@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@betodealmeida betodealmeida added the risk:db-migration PRs that require a DB migration label Nov 14, 2021
@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

18 similar comments
@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2021

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2022

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2022

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2022

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2022

⚠️ @cccs-RyanS Your base branch master has just also updated superset/migrations.

Please consider rebasing your branch to avoid db migration conflicts.

@villebro
Copy link
Member

villebro commented Mar 7, 2022

Closing in favor of #18794

@villebro villebro closed this Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
risk:db-migration PRs that require a DB migration size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants