-
Notifications
You must be signed in to change notification settings - Fork 23
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
Checking pixellimit for r.import commands #491
Conversation
May I suggest adding a test actinia-core/tests/test_geodata_import.py Line 170 in 852f159
The test may then execute two imports:
|
@neteler Good hint, I will add a test and thanks for the example data. |
Co-authored-by: Markus Neteler <[email protected]>
Co-authored-by: Markus Neteler <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, well done!
Please rename the tests like suggested or in another way, a negation in the title makes me expect a 400.
Other than that I "only" have ideas for more tests like using the importer, testing the same with ephemeral processing, ... but no blocker for me to be merged like this because tests for two cases exist.
This method will raise an AsyncProcessError exception | ||
|
||
""" | ||
rimport_inp = [x for x in process_executable_params if "input=" in x][ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, an importer command would already be translated to r.import
, right? So the following would be parsed correctly because it is already transformed?:
{
"id": "importer_1",
"module": "importer",
"inputs": [
{
"import_descr": {
"source": "https://raw.githubusercontent.com/mmacata/pagestest/gh-pages/bonn.geojson",
"type": "vector"
},
"param": "map",
"value": "polygon"
}
]
},
(Maybe add an importer PC to the tests below?) 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is already transformed at this point. I also added a test for the importer
src/actinia_core/processing/actinia_processing/ephemeral_processing.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Carmen Tawalika <[email protected]>
Co-authored-by: Carmen Tawalika <[email protected]>
…ssing.py Co-authored-by: Carmen Tawalika <[email protected]>
Check the current
r.import
command against the user cell limit (withinephemeral_processing.py
) + added tests for this checkPR additionally contains small linting changes:
test_download_cache.py
(line too long)