m9brady / eodms-api-client

EODMS API Client for Python
https://m9brady.github.io/eodms-api-client/
MIT License
18 stars 5 forks source link

Order ID appears to be off-by-one #3

Closed dsjstc closed 2 years ago

dsjstc commented 2 years ago

Describe the bug The request IDs on the website seem to be 1 higher than the one returned by the client.query() method.

To Reproduce Query and order products from RCM. Navigate to the portal -> My Orders. Compare the Order ID column from the portal to the order ID in the query results.

Expected behavior Order ids should match.

Additional context Package version: 1.0.1

dsjstc commented 2 years ago

NB that the download() method now seems to expect order ID, rather than record ID... at least for the RCM product.

m9brady commented 2 years ago

Hi, thanks for the report.

I have a warning in the docs that may pertain to this: https://eodms-api-client.readthedocs.io/en/latest/examples.html#command-line-interface-cli but there may be a case to be made of making this warning a bit more obvious to the user.

It seems that submitting an order to the EODMS API will return a seemingly-valid Order Item ID but in fact, that is not the correct ID value and it is up to the user to check against the delivery email that is sent on order completion for the "real" ID. It isn't immediately clear how these ID values are generated and why the values that get sent as a response to the API do not match what is delivered in the email, but that may be something that has to be solved on NRCan's side.

I wasn't able to figure out a way around this issue programmatically and I don't think it's as easy as just incrementing the API-returned OrderIDs by 1. However, I'd be happy to be proven wrong on this with a reproducible example.

Edit: as for the download method, it might be better to indicate in the docstrings that it expects Order ID rather than Record ID....

m9brady commented 2 years ago

I just ran a test with 1 RCM scene and what appears to happen is the following:

  1. EODMS API accepts order given a scene's RecordID (this RecordID was taken from an EodmsAPI.query() result geojson)
  2. EODMS API returns an Item ID (hence ID1) for the scene when the order is submitted using EodmsAPI.order()
  3. EODMS system does its thing
  4. EODMS finishes its work and generates a new Item ID (hence ID2) for the same scene, but in an AVAILABLE FOR DOWNLOAD state
  5. ID1 is still a valid ID but it shows up as EXPANDED state in the EODMS web interface and does not have any download URLs attached to it
  6. ID2 is what gets sent in the "your product is ready" email from EODMS and is used by this package for downloading

In this example, ID2 is literally ID1 + 1 but I worry that if we just increment by 1 then the possibility for a collision between our order and someone else's order is possible. I doubt that it would be possible to download another user's order but you still wouldn't get what you want, which is the image(s) you asked for.

I don't fully understand why EODMS generates 2 Item IDs for the same image, and I'm not sure what the best course of action is besides:

m9brady commented 2 years ago

👀 now I understand what you mean re: OrderId instead of RecordID - it seems that the EODMS API finally allows for querying by OrderID to check status of items! For so long that was not the case and it would just return the last 100 ordered images so this is great news.

m9brady commented 2 years ago

Hi @dsjstc I just pushed a new version v1.1.0 that uses the OrderID instead of each RCM scene's ItemID. Can you try upgrading and see if it fixes your issue?

CLI:

eodms -c RCM --download-id <orderid>

Python REPL/IPython

from eodms_api_client import EodmsAPI
client = EodmsAPI(collection='RCM')
client.download(order_ids=<orderid>)
dsjstc commented 2 years ago

Success confirmed with order_id, this is spectacularly awesome. Thanks so much, and glad my garbled comment was of use to you!

m9brady commented 2 years ago

Awesome 🥳 thanks for reporting and glad that the package is useful to you!