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

Fix error when retrieving GeoZones #2570

Closed
wants to merge 4 commits into from

Conversation

mdamien
Copy link
Contributor

@mdamien mdamien commented Nov 8, 2020

The issue is that the script try to write to /udata/fs/tmp/ but the tmp directory does not exist

Related issue: opendatateam/docker-udata#197

I'm not sure it's the best place to ensure the /tmp/ directory is there, maybe directly at the initialization phase in udata/core/storages/__init__.py

@mdamien mdamien changed the title fix error when retrieving GeoZones Fix error when retrieving GeoZones Nov 8, 2020
@quaxsze
Copy link
Contributor

quaxsze commented Nov 9, 2020

This solution might work but creates a specific behavior which differs from the rest of the storages.
Flask-fs uses ensure_path method on its write() and save() methods. The flask-fs way would be to download the file using the chunk's storage and use its save() method.
Another simpler way would be to use python's temporary file feature instead of using a Flask-fs storage.

@mdamien
Copy link
Contributor Author

mdamien commented Nov 9, 2020

I could do:

        log.info('Downloading GeoZones bundle: %s', filename)
        with tmp.open(GEOZONE_FILENAME, 'w') as f:
            pass
        filename, _ = urlretrieve(filename, tmp.path(GEOZONE_FILENAME))

So it works with all backends

@quaxsze
Copy link
Contributor

quaxsze commented Nov 9, 2020

It's actually even more hacky than the previous one 😅
When I mentioned other storages, I meant that they download files using the chunk way and the ensure_path is implicitly called within the save function.
The solution is to either use a storage provided by Flask-FS and to proceed the way the librairy is designed for, or if this is over complicated for the wanted result, to access the system tmp by using python`s tempfile instead of using flask-fs.

@mdamien
Copy link
Contributor Author

mdamien commented Nov 10, 2020

@quaxsze ahah, this time I pushed something you will like I think, downloading the chunk way with urlopen

@quaxsze
Copy link
Contributor

quaxsze commented Nov 13, 2020

Sorry my previous comment was not very clear.
When I mentioned the chunks way I was thinking of using chunk's storage like in the handle_upload function.
It might seem a bit over complicated that is why mentioned accessing the system tmp by using python`s tempfile instead of using flask-fs.

@mdamien
Copy link
Contributor Author

mdamien commented Nov 19, 2020

@quaxsze pushed a simple solution with tempfile.NamedTemporaryFile, let me know if it's what you're looking for

@ThibaudDauce
Copy link
Contributor

I don't think this bug exists anymore. Feel free to open an issue or a new PR if it's still the case.

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.

3 participants