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

Code snippets for all APIs #939

Draft
wants to merge 46 commits into
base: maint-2.0
Choose a base branch
from

Conversation

kevinlacaille
Copy link
Contributor

@kevinlacaille kevinlacaille commented Apr 26, 2023

Related Issue(s):

Closes #540 #937

Proposed Changes:

For inclusion in changelog (if applicable):

  1. Code snippets for Orders API, Data API, and Subscriptions API.

Not intended for changelog:

Diff of User Interface

Old behavior:
N/A

New behavior:
N/A

PR Checklist:

  • This PR is as small and focused as possible
  • If this PR includes proposed changes for inclusion in the changelog, the title of this PR summarizes those changes and is ready for inclusion in the Changelog.
  • I have updated docstrings for function changes and docs in the 'docs' folder for user interface / behavior changes
  • This PR does not break any examples or I have updated them

(Optional) @mentions for Notifications:
@jreiberkyle

@kevinlacaille kevinlacaille marked this pull request as ready for review April 26, 2023 19:39
@kevinlacaille kevinlacaille marked this pull request as draft April 26, 2023 19:42
@kevinlacaille kevinlacaille changed the title Code snippets for all APIs, tested and MD integrated Code snippets for all APIs May 1, 2023
@kevinlacaille kevinlacaille changed the base branch from 2.0.1 to main May 5, 2023 20:14
@kevinlacaille kevinlacaille modified the milestones: 2.1: Major improvements, 2.0.x: Bug fixes and Docs May 11, 2023
@kevinlacaille kevinlacaille linked an issue May 11, 2023 that may be closed by this pull request
@sgillies
Copy link
Contributor

@kevinlacaille I've realized a complication for testing against the production APIs: the test runner can only access saved searches and orders that it has made using the same credentials. It looks to me like your tests rely on your own saved searches and orders and that's not going to work.

So it looks like you'll have to have a snippet to make a search, then get that search's id to go back to it. Same for orders.

@kevinlacaille kevinlacaille changed the base branch from main to maint-2.0 June 14, 2023 19:12
@sgillies
Copy link
Contributor

@kevinlacaille the main source of errors in the new tests is that the clients are pointed at the production API while the credentials stored in GitHub are for staging. See https://github.com/planetlabs/planet-client-python/actions/runs/5525726227/jobs/10079664599#step:5:616.

I think we should reconsider running these snippets against our API. It's not very complicated for the Data API. But it gets more complicated for APIs that are stateful. Is it going to be feasible to keep making and downloading orders every time these tests run? Not to mention subscriptions, which need a cloud destination.

@kevinlacaille
Copy link
Contributor Author

NOTE: Maybe add in CONTRIBUTING.md on how to add to the snippets if a new function is added or if functionality changes.

@asonnenschein asonnenschein added the 2024-revisit SDK committee reviewed and marked for follow-up label Jun 13, 2024
@tbarsballe tbarsballe modified the milestones: 2.0.x: Bug fixes and Docs, 2.x.x: Wishlist Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants