reenabled ghost tables (do not merge) #874
Conversation
there are some things to review: - how the ghost tables task is launched - will it work with multiuser - the user specs where not working
there are some things to review: - how the ghost tables task is launched - will it work with multiuser - the user specs where not working
|
@rochoa could you check in your local install? |
|
I checked it in my local installation and previous issues are fixed. Thanks. |
|
@Kartones code review please. After that @nick13jaremek could you test in staging please? |
| @@ -18,6 +18,12 @@ def render_jsonp obj, status = 200, options = {} | |||
|
|
|||
| def link_ghost_tables | |||
| return true unless current_user.present? | |||
| current_user.link_ghost_tables | |||
| if current_user.ghost_tables_enabled && current_user.search_for_modified_table_names | |||
There was a problem hiding this comment.
I'm a bit afraid that always checking this could be a performance hit, as the check will be executed on every API call (we're crossing at minimum two queries, user real tables and metadata tables).
We can just deploy it and watch the stats, or maybe have a user flag with the timestamp of last ghost tables check, so it doesn'checks and adds again if not past X time (probably just 1h would be enough).
|
Apart from my concern of constantly checking for ghost tables on API calls, the code looks fine +1 |
|
@Kartones we could limit to dashboard ones (also this is not enabled for all the users) |
|
Well, even in dashboard ones I'd check stats, but that would be much less, yes (and I'd perfectly release it without any timestamp flag) |
|
"timestamp flag" ? |
|
@nick13jaremek don't test this until I change to check this only on dashboard api calls |
|
A flag to mark last "register sweep" and avoid any further check until past X time. But for now restrict to only dashboard API calls and we're good to go, can be changed without problems |
this endpoint is called from table too but is ok
|
nice! |
|
waiting for CI, will merge it |
|
cannot avoid to do a ping on this. :) |
|
@saleiva Tests have passed successfully ;) |
|
will deploy tomorrow guys 👯 💃 👯 |
|
@javisantana tomorrow? cagao ;) |
|
Hmm... have you seen this before?
I get it on trying to register a ghost table select%20cdb_cartodbfytable(%27ghost_testi%27) on andrew.cartodb |
|
@andrewxhill yes, that's a problem we have sometimes but it's not related to the cartodby, I will file another ticket: #1150 |
there are some things to review:
test to be done
before start
be sure
ghost_tables_enabledis true for the user you are testingcreate table
the table should be there
drop table
the table should not be there
rename table
the table should not be there
create these tests using a MU account
ideally these tests can be performed using the SQL API and the REST API so they can be automated using python/bash