-
Notifications
You must be signed in to change notification settings - Fork 6
1111 return stac11 items #411
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
base: master
Are you sure you want to change the base?
Conversation
…TAC items" This reverts commit 1109ec4. This added top-level (Collection) links to item-level links implicitly so not right conceptually. Support dedicated derived_from document only for "STAC 1.1 items" (implemented in #411). Open-EO/openeo-geopyspark-driver#1278
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.
Did a quick review; added some points of interest inline.
I can help you with adding tests to test_views.py
for these paths:
/jobs/<job_id>/results
/jobs/<job_id>/results/items11/<item_id>
and/or their signed counterparts.
ml_model_metadata = metadata | ||
|
||
if len(result_metadata.items) > 0 : | ||
for item_id, metadata in result_metadata.items.items(): |
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.
for item_id, metadata in result_metadata.items.items(): | |
for item_id in result_metadata.items.keys(): |
if item_id == DriverMlModel.METADATA_FILE_NAME: | ||
return _download_ml_model_metadata(job_id, item_id, user_id) |
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.
Will this ever be the case?
) | ||
|
||
if item_id not in metadata.items: | ||
raise OpenEOApiException("Item with id {item_id!r} not found in job {job_id!r}".format(item_id=item_id, job_id=job_id)) |
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.
Return a 404.
raise OpenEOApiException("Item with id {item_id!r} not found in job {job_id!r}".format(item_id=item_id, job_id=job_id)) | ||
item_metadata = metadata.items.get(item_id,None) | ||
|
||
resp = jsonify(item_metadata) |
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.
Basic, will probably require more work e.g. sign its asset URLs?
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.
Added some test skeletons to complete.
Ideally the URLs have the original /items
instead of /items11
.
Let me know if anything is not clear.
method_start = method_start + "11" | ||
if not signer: | ||
return url_for(".get_job_result_item", job_id=job_id, item_id=item_id, _external=True) | ||
return url_for(method_start, job_id=job_id, item_id=item_id, _external=True) |
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.
Is it possible to avoid a new endpoint (/jobs/<job_id>/results/items11/<item_id>
)? I suppose the original _get_job_result_item
method can also check whether the item ID corresponds to either:
- an item derived from an asset (old case) or;
- an actual item generated in the batch job (new case aka stac11).
) | ||
|
||
def test_get_job_results_from_stac_1_1_items(self, api110): | ||
# TODO: use UrlSigner |
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.
Address TODOs.
assert item_link["href"] == ... | ||
|
||
def test_get_stac_1_1_item(self): | ||
# TODO: use UrlSigner |
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.
Address TODOs.
Open-EO/openeo-geopyspark-driver#1111
TODO:
Example item entry: