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.

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

Overpass API import script #37

Merged
merged 6 commits into from
Jan 21, 2022
Merged

Overpass API import script #37

merged 6 commits into from
Jan 21, 2022

Conversation

mvaaltola
Copy link
Contributor

@mvaaltola mvaaltola commented Nov 26, 2021

Add script for importing data from Overpass API to PostGIS.

todos:

  • use this instead of import_osm.sh
  • remove unneeded osm2pgsql dependency from image
  • merge conflict in import_flickr
  • pin test/dev dependencies using pip-tools (in Pin dependencies with pip-tools #34)
  • separate tests to test directory (wontfix. larger refactoring needed)
  • run tests automatically in GH (in CI/CD Pipeline #24)

@mvaaltola mvaaltola marked this pull request as draft November 26, 2021 10:44
@mvaaltola mvaaltola changed the title WIP: Overpass api Overpass API import script Nov 26, 2021
@mvaaltola mvaaltola marked this pull request as ready for review November 26, 2021 11:53
@mvaaltola mvaaltola requested a review from Rikuoja November 26, 2021 11:55
scripts/docker-compose.yml Outdated Show resolved Hide resolved
scripts/import_osm.py Outdated Show resolved Hide resolved
scripts/import_osm.py Outdated Show resolved Hide resolved
scripts/import_osm.py Outdated Show resolved Hide resolved
scripts/import_osm.py Outdated Show resolved Hide resolved
scripts/import_osm.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Rikuoja Rikuoja left a comment

Choose a reason for hiding this comment

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

Looks excellent! Added some comments and things to fix before merging.

The details about file paths, testing etc. can be fixed also after merging.

scripts/import_osm.py Outdated Show resolved Hide resolved
scripts/import_osm.py Outdated Show resolved Hide resolved
import.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Rikuoja Rikuoja left a comment

Choose a reason for hiding this comment

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

Log path needs fixing, but otherwise this looks very good!

util.py Outdated
import sys
from pathlib import Path

LOG_PATH = Path('/app/logs')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm afraid we will have to use

log_file = os.path.join(os.path.dirname(__loader__.path), IMPORT_LOG_PATH, f"{city}.log")

or something similar here, so that this setup works

  1. inside the container
  2. outside the container, with a totally different directory inside the repo,
  3. when called on the command line in the top directory
  4. when called by flask.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Otherwise I will get

FileNotFoundError: [Errno 2] No such file or directory: '/app/logs/helsinki.log'

when running locally.

@mvaaltola mvaaltola requested a review from Rikuoja January 21, 2022 07:39
@Rikuoja Rikuoja merged commit 60eba2a into main Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants