-
Notifications
You must be signed in to change notification settings - Fork 17.4k
Add synctable command into caravel #969
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
Changes from all commits
886a160
3a28a5d
18f845c
3f7a84f
ee7cd87
2a383f0
2438334
5db3c69
a165f43
21cb7cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,6 +125,35 @@ def refresh_druid(): | |
| "[" + cluster.cluster_name + "]") | ||
| session.commit() | ||
|
|
||
| @manager.option( | ||
| '-p', '--prefix', default='data_', | ||
| help="Sync Table Prefix") | ||
| def synctable(prefix): | ||
| ''' Sync DB Table with Caravel Table''' | ||
| existing_tables = [] | ||
| for row in db.session.query(caravel.models.SqlaTable).all(): | ||
| existing_tables += [(row.database.database_name, row.name)] | ||
| all_tables = [] | ||
| for caravel_db in db.session.query(caravel.models.Database).all(): | ||
| for table_name in caravel_db.get_sqla_engine().table_names(): | ||
| all_tables += [(caravel_db.database_name, table_name)] | ||
|
|
||
| all_tables = [row for row in all_tables if row[1].startswith(prefix)] | ||
|
|
||
| need_insert = list(set(all_tables) - set(existing_tables)) | ||
|
|
||
| for db_name, table_name in need_insert: | ||
| tbl = caravel.models.SqlaTable(table_name=table_name) | ||
| tbl.description = "" | ||
| tbl.is_featured = False | ||
| tbl.database = db.session.query(caravel.models.Database).filter_by(database_name=db_name).first() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think you can query this once outside the loop
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean #152 ? |
||
| db.session.merge(tbl) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. db.session.add(tbl) should be enough |
||
| db.session.commit() | ||
| tbl.fetch_metadata() | ||
|
|
||
| print("[{db}] {table} insert success.".format(db=db_name, table=table_name)) | ||
|
|
||
| print("[{}] Sync table complete.".format(len(need_insert))) | ||
|
|
||
| if __name__ == "__main__": | ||
| manager.run() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd make the prefix not required and empty by default so one can insert all the tables of a database if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is not good because some table in caravel db store user,permission...etc data, they don't need insert to caravel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but 'data_' is completely arbitrary, if you don't want to read all the tables fine, just don't add a default that would work only for you :)