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.

Sign up for GitHub

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

Add synctable command into caravel #977

Closed
wants to merge 19 commits into from
Closed

Add synctable command into caravel #977

Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
886a160
add redis cache hint to docs
linroex Jul 30, 2016
3a28a5d
add synctable cmd and get need_insert tables
linroex Aug 17, 2016
18f845c
synctable done
linroex Aug 17, 2016
3f7a84f
Merge branch 'master' of github.com:linroex/caravel
linroex Aug 17, 2016
ee7cd87
add synctable desc
linroex Aug 17, 2016
2a383f0
Merge branch 'synctable'
linroex Aug 17, 2016
2438334
fine tune
linroex Aug 18, 2016
5db3c69
[synctable] fix bug: multi insert table
linroex Aug 18, 2016
a165f43
[synctable] add hint console output
linroex Aug 18, 2016
21cb7cf
Merge branch 'master' into master
linroex Aug 18, 2016
eec6c5f
use defaultdict to update code
linroex Aug 19, 2016
0239ca3
Merge Master
linroex Aug 19, 2016
11c7c01
[synctable] use add instead of merge
linroex Aug 19, 2016
f70ad9b
Merge branch 'master' into master
linroex Aug 19, 2016
c57200e
specifies sync db
linroex Aug 19, 2016
d0cb663
[synctable] add database hint text
linroex Aug 19, 2016
06a65d0
add warning when sync all table and set prefix is empty str
linroex Aug 19, 2016
6aa9d3e
check is database exists
linroex Aug 19, 2016
e264d20
[synctable] fine tune
linroex Aug 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions caravel/bin/caravel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,35 @@ def refresh_druid():
"[" + cluster.cluster_name + "]")
session.commit()

@manager.option(
'-p', '--prefix', default='data_',
Copy link
Contributor

Choose a reason for hiding this comment

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

just default to None and if it's None just don't filter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think no filter is not good, because main database (for caravel system use) have many table not store data, It doesn't need sync.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, first thing is that you should not install caravel on the same database of your data. Second the feature should be generic and not tied to your own usage :)

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))
Copy link
Contributor

Choose a reason for hiding this comment

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

If you transform this list of tuples in a dict you can save some work later:

from collections import defaultdict

# can't find a better name atm :)
insert_dict = defaultdict(list)
for db_name, table_name in need_insert:
    insert_dict[db].append(table)

for db_name, tables in insert_dict.items():
    database = db.session.query(caravel.models.Database).filter_by(database_name=db_name).first()
    for table in tables:
       tbl = ...
       tbl.database = database
       ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, thank you very much!


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()
db.session.merge(tbl)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need merge here instead of add?

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()