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

feat: fetch API specifications for stand-alone API Explorer #789

Merged
merged 33 commits into from
Aug 10, 2021

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Aug 9, 2021

Changes the RunIt forms to make some of the features of RunIt available without requiring configuration and OAuth authentication to a Looker server:

  • ConfigForm
  • LoginForm
  • RequestForm

This also adds recognition of headless: true|false to a versions.json payload to make it easier/more dynamic to configure the display mode for the stand-alone API Explorer. This can be used for the API Explorer instance that runs on https://developers.looker.com

jkaster and others added 28 commits May 1, 2021 02:51
- We can't check spec key any more because specs will vary with different versions payloads

- The React error boundary should help inform users of the stand-alone version that API 4.0 is required for Run functions due to CORS requirements
BROKEN CODE RIGHT NOW
# Conflicts:
#	packages/extension-api-explorer/src/ExtensionApiExplorer.tsx
- Tooltips and streamlining
@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2021

APIX Tests

    1 files    75 suites   2m 33s ⏱️
282 tests 269 ✔️ 13 💤 0 ❌
301 runs  288 ✔️ 13 💤 0 ❌

Results for commit c5a8273.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2021

Codegen Tests

  1 files    6 suites   24s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit c5a8273.

Now the error doesn't get cleared. Trying to figure out state management issue
@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2021

Codegen Tests

  1 files    6 suites   22s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 85c2546.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2021

APIX Tests

    1 files    75 suites   2m 44s ⏱️
282 tests 269 ✔️ 13 💤 0 ❌
301 runs  288 ✔️ 13 💤 0 ❌

Results for commit 85c2546.

Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor things. Taking for a spin so maybe more feedback

@@ -49,8 +49,8 @@ export const ApiSpecSelector: FC<ApiSpecSelectorProps> = ({
description: spec.status,
}))

const handleChange = (specKey: string) => {
specDispatch(selectSpec(specs, specKey))
const handleChange = async (specKey: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the async needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's something you added on 7/23/21, probably when working on the fetching spec on demand. Going to leave it in until you have more feedback.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can removed. I vaguely remember that I had logic to make an aync call here but ended up moving that code to useEffect elsewhere. Forgot to remove the async

packages/api-explorer/src/reducers/spec/utils.spec.ts Outdated Show resolved Hide resolved
packages/api-explorer/src/reducers/spec/utils.spec.ts Outdated Show resolved Hide resolved
packages/api-explorer/src/reducers/spec/utils.spec.ts Outdated Show resolved Hide resolved
packages/run-it/src/components/ConfigForm/ConfigForm.tsx Outdated Show resolved Hide resolved
packages/run-it/src/components/RequestForm/RequestForm.tsx Outdated Show resolved Hide resolved
@bryans99
Copy link
Collaborator

bryans99 commented Aug 9, 2021

Be nice to provide feed back on successful verification

@github-actions
Copy link
Contributor

Codegen Tests

  1 files    6 suites   27s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 114c785.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    75 suites   2m 35s ⏱️
282 tests 268 ✔️ 13 💤 1 ❌
301 runs  287 ✔️ 13 💤 1 ❌

For more details on these failures, see this check.

Results for commit 114c785.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    75 suites   2m 30s ⏱️
282 tests 269 ✔️ 13 💤 0 ❌
301 runs  288 ✔️ 13 💤 0 ❌

Results for commit b7acdf3.

@github-actions
Copy link
Contributor

Codegen Tests

  1 files    6 suites   29s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit b7acdf3.

Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove async, consider changing message. After that we good to go.

packages/run-it/src/components/ConfigForm/ConfigForm.tsx Outdated Show resolved Hide resolved
@@ -49,8 +49,8 @@ export const ApiSpecSelector: FC<ApiSpecSelectorProps> = ({
description: spec.status,
}))

const handleChange = (specKey: string) => {
specDispatch(selectSpec(specs, specKey))
const handleChange = async (specKey: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can removed. I vaguely remember that I had logic to make an aync call here but ended up moving that code to useEffect elsewhere. Forgot to remove the async

@jkaster jkaster requested a review from bryans99 August 10, 2021 16:33
@github-actions
Copy link
Contributor

APIX Tests

    1 files    75 suites   2m 2s ⏱️
282 tests 269 ✔️ 13 💤 0 ❌
301 runs  288 ✔️ 13 💤 0 ❌

Results for commit ab4753f.

@github-actions
Copy link
Contributor

Codegen Tests

  1 files    6 suites   21s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit ab4753f.

Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkaster jkaster merged commit f7be1fb into main Aug 10, 2021
@jkaster jkaster deleted the jk/apix_spec_fetch branch August 10, 2021 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants