Synchronization overwrite mode for geocoding only #14991
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is just an experiment. It may require far more than the automated tests in this repository, yet worth a try.
This also solves an inconsistency between first and subsequent syncs
Table indices and privileges need not be replicated now since the table is preserved
houndci-bot
reviewed
Jun 25, 2019
|
There is a slight difference between the previous overwrite by replacement and the new one: The |
# Conflicts: # lib/sql
houndci-bot
reviewed
Jun 26, 2019
spec/requests/carto/api/custom_visualizations_controller_spec.rb
Outdated
Show resolved
Hide resolved
spec/requests/carto/api/custom_visualizations_controller_spec.rb
Outdated
Show resolved
Hide resolved
app/controllers/carto/api/public/custom_visualizations_controller.rb
Outdated
Show resolved
Hide resolved
|
Notice: before merging this, a new release of the extension is needed. Don't forget to update the lib/sql submodule and the version references in the code before merging. |
This is for testing purposes (with the branch in the lib/sql submodule)
houndci-bot
reviewed
Jun 26, 2019
| sanitize(column_names) | ||
| end | ||
|
|
||
|
|
|
|
||
| sanitize(column_names) | ||
| # Already done by Table#sanitize_columns in app/models/table.rb | ||
| #sanitize_columns! |
| @@ -187,6 +267,10 @@ def import_cleanup(schema_name, table_name) | |||
| qualified_table_name = "\"#{schema_name}\".#{table_name}" | |||
|
|
|||
| user.db_service.in_database_direct_connection(statement_timeout: STATEMENT_TIMEOUT) do |user_database| | |||
|
|
|||
| # For consistency with regular imports, also eases testing | |||
| Table.sanitize_columns(table_name, {database_schema: schema_name, connection: user_database}) | |||
There was a problem hiding this comment.
Space inside { missing.
Redundant curly braces around a hash parameter.
Space inside } missing.
houndci-bot
reviewed
Jun 26, 2019
jgoizueta
force-pushed
the
sync-api-table-sync
branch
from
ee142c1
to
9f2814b
Compare
# Conflicts: # app/controllers/home_controller.rb # app/models/user/db_service.rb # lib/sql
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes minimal modifications to choose between overwriting by replacement or synchronization.
The the new sync mode is used only for tables that have a
carto_geocode_hashcolumn, i.e. tables processed by the geocoder analysis. This is so to be able to put this in production in the safest way: regular syncs, in particular all existing syncs shouldn't be affected.This would benefit from cleaning up Table/TableSetup/Adapter redundancies, specially import_cleanup, but we'll keep that separate.
Note that this build upon the work in #14937