-
Notifications
You must be signed in to change notification settings - Fork 96
Replacing python quickstart with a CLI one #472
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
Conversation
@jreiberkyle & @sgillies - can you help me with the proper CLI command to wait for an order and download it when ready? The one example I have is:
Which isn't right since it 'waits' on both calls. I tried out some variations with download but none seemed to do things right. If one of you could give the proper command here I can try it out and update the docs. |
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.
thank you for adding the auth stuff here. that was needed sorely. feel free to take my advice here as you wish. If you'd like, we can just update the create order
content when the CLI change is implemented.
README.md
Outdated
|
||
``` | ||
% planet orders create --name my-first-order --id <scene ids> \ |
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.
The creation of the request and the creation of the order will be broken apart (this work is currently ticketed).
We can either walk the user through the process of creating a request or we can just provide them with a little request json example and go straight to creating the order. My preference is the latter since request creation is something that could evolve. Up to you.
Option 1:
My two suggestions for improvement here would be to add actual scene ids and to change the item type to PSScene since it is our current focus for item type and also some people may not have access to SkySat
$ planet orders request --name my-first-order \
--id 20210821_161925_80_2453,20210821_161923_32_2453 \
--item-type PSScene \
--bundle visual
Option 2:
request.json (matches Option 1)
{
"name":"my-first-order",
"products":[
{
"item_ids":[
"20210821_161925_80_2453",
"20210821_161923_32_2453"
],
"item_type":"PSScene",
"product_bundle":"visual"
}
]
}
and then:
$ planet orders create request.json
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.
add actual scene ids
I feel like there's no scene ids that most or even many people would have? That was my hesitation, putting in a command that wouldn't work for most people because they don't have access. Putting a placeholder I hoped would make people actually try to change it to something that works. Perhaps we could use one that works in the dev trial at least? I wish open california still existed... Or that Planet admin system had an easy way to add data that anyone can access - could at least put the open data we have in STAC there.
PSScene since it is our current focus for item type
What do you mean by this? Just that we are pushing this as the 'new' thing?
I don't feel strongly, so am fine to switch to skysat.
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.
Oh, and I definitely prefer option 1, give people the CLI instead of making them write json. But we don't have the 'request' thing implemented yet, right? I'd say we just merge this in and change it when that exists.
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 merging now and fixing later: sounds good!
for scene ids: yeah, it would be awesome for us to have a 'starter pack' of scenes anyone with valid planet auth could access. In fact, is there a minimum set already established? My guess is 'no' but it would be great to find out.
PSScene: My thinking is that the primary sensor that people will use is a PS sensor, and all of the PS sensors are wrapped up in PSScene. Basically, let's align our help with what planet is messaging. Since PSScene was just released I figure it's probably being highlighted by planet.
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.
I don't believe there is a minimum set established. Maybe @sarasafavi knows? I think until there is one I'll just leave it in with a placeholder. Sounds good on PSScene.
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
% to $ Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
Ok, this is ready for final review. I can update the docs when #366 lands, but prefer to just get this one in. |
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.
Lots of little nits but ultimately this looks great. You may want to remove the *.json
files that got added along with the README
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
Co-authored-by: Jennifer Reiber Kyle <jreiberkyle@users.noreply.github.com>
Ok, I think I addressed everything, thanks for the great review! |
yep looks great! |
Cool, merging in. |
This removes the python quickstart, which is available elsewhere in the docs, with a CLI one, as that's what we want to emphasize. This closes #453