Skip to content

Commit

Permalink
Merge pull request #216 from GeoNode/ISSUE_215
Browse files Browse the repository at this point in the history
Pipe the dump of the ogr2ogr command when using PgPool
  • Loading branch information
giohappy authored Dec 1, 2023
2 parents 1fbcb73 + bb3c9fd commit 2f4b6fd
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 22 deletions.
5 changes: 2 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,8 @@ IMPORTER_PUBLISHING_RATE_LIMIT= # default 5
IMPORTER_RESOURCE_CREATION_RATE_LIMIT= # default 10
IMPORTER_RESOURCE_COPY_RATE_LIMIT = # default 10
# https://gdal.org/drivers/vector/pg.html#configuration-options
DISABLE_PG_COPY_OGR2OGR= # If TRUE disable the usage of COPY during the import in postgres. Disable it can affect the performance
# https://github.com/OSGeo/gdal/issues/8674
OGR2OGR_COPY_WITH_DUMP = If true, will pipe the PG dump to psql.
```

## Troubleshooting
Expand Down
15 changes: 7 additions & 8 deletions importer/handlers/common/tests_vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,9 @@ def test_import_with_ogr2ogr_with_errors_should_raise_exception(self, _open):
shell=True, # noqa
)

@patch.dict(os.environ, {"DISABLE_PG_COPY_OGR2OGR": "True"}, clear=True)
@patch.dict(os.environ, {"OGR2OGR_COPY_WITH_DUMP": "True"}, clear=True)
@patch("importer.handlers.common.vector.Popen")
def test_import_with_ogr2ogr_without_errors_should_call_the_right_command_if_copy_is_disabled(
def test_import_with_ogr2ogr_without_errors_should_call_the_right_command_if_dump_is_enabled(
self, _open
):
_uuid = uuid.uuid4()
Expand All @@ -283,9 +283,8 @@ def test_import_with_ogr2ogr_without_errors_should_call_the_right_command_if_cop
self.assertEqual(str(_uuid), execution_id)

_open.assert_called_once()
_open.assert_called_with(
f'/usr/bin/ogr2ogr --config PG_USE_COPY NO -f PostgreSQL PG:" dbname=\'geonode_data\' host=localhost port=5434 user=\'geonode\' password=\'geonode\' " "{self.valid_files.get("base_file")}" -nln alternate "dataset"',
stdout=-1,
stderr=-1,
shell=True, # noqa
)
_call_as_string = _open.mock_calls[0][1][0]

self.assertTrue('-f PGDump /vsistdout/' in _call_as_string)
self.assertTrue('psql -d' in _call_as_string)
self.assertFalse('-f PostgreSQL PG' in _call_as_string)
40 changes: 29 additions & 11 deletions importer/handlers/common/vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,17 +154,28 @@ def create_ogr2ogr_command(files, original_name, ovverwrite_layer, alternate):
Define the ogr2ogr command to be executed.
This is a default command that is needed to import a vector file
"""
_datastore = settings.DATABASES['datastore']

disable_pg_copy = ast.literal_eval(os.getenv("DISABLE_PG_COPY_OGR2OGR", "False"))
options = f"--config PG_USE_COPY {'NO' if disable_pg_copy else 'YES'} "

options += (
"-f PostgreSQL PG:\" dbname='%s' host=%s port=%s user='%s' password='%s' \" "
% (_datastore['NAME'], _datastore['HOST'], _datastore.get('PORT', 5432), _datastore['USER'], _datastore['PASSWORD'])
)
_datastore = settings.DATABASES["datastore"]

options = "--config PG_USE_COPY YES"
copy_with_dump = ast.literal_eval(os.getenv("OGR2OGR_COPY_WITH_DUMP", "False"))

if copy_with_dump:
# use PGDump to load the dataset with ogr2ogr
options += ' -f PGDump /vsistdout/ '
else:
# default option with postgres copy
options += (
" -f PostgreSQL PG:\" dbname='%s' host=%s port=%s user='%s' password='%s' \" "
% (
_datastore["NAME"],
_datastore["HOST"],
_datastore.get("PORT", 5432),
_datastore["USER"],
_datastore["PASSWORD"],
)
)
options += f'"{files.get("base_file")}"' + " "
# options += "-lco DIM=2 "

options += f'-nln {alternate} "{original_name}"'

if ovverwrite_layer:
Expand Down Expand Up @@ -764,7 +775,7 @@ def rollback(
steps = self.ACTIONS.get(action_to_rollback)
if rollback_from_step not in steps:
logger.info("Step not found, skipping")
return
return
step_index = steps.index(rollback_from_step)
# the start_import, start_copy etc.. dont do anything as step, is just the start
# so there is nothing to rollback
Expand Down Expand Up @@ -942,6 +953,12 @@ def import_with_ogr2ogr(
options = orchestrator.load_handler(handler_module_path).create_ogr2ogr_command(
files, original_name, ovverwrite_layer, alternate
)
_datastore = settings.DATABASES["datastore"]

copy_with_dump = ast.literal_eval(os.getenv("OGR2OGR_COPY_WITH_DUMP", "False"))

if copy_with_dump:
options += f" | PGPASSWORD={_datastore['PASSWORD']} psql -d {_datastore['NAME']} -h {_datastore['HOST']} -p {_datastore.get('PORT', 5432)} -U {_datastore['USER']} -f -"

commands = [ogr_exe] + options.split(" ")

Expand All @@ -951,6 +968,7 @@ def import_with_ogr2ogr(
stderr is not None
and stderr != b""
and b"ERROR" in stderr
and b"error" in stderr
or b"Syntax error" in stderr
):
try:
Expand Down

0 comments on commit 2f4b6fd

Please sign in to comment.