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

Rewrite QA client #3262

Closed
justinlittman opened this issue Oct 28, 2021 · 7 comments · Fixed by #3312 or #3342
Closed

Rewrite QA client #3262

justinlittman opened this issue Oct 28, 2021 · 7 comments · Fixed by #3312 or #3342
Assignees

Comments

@justinlittman
Copy link
Contributor

Currently our QA client is implemented using swagger-client, which dynamically builds a client based on a local openapi spec. This is problematic because:

  • The client code is complex and hard to make changes.
  • The local openapi spec is not maintained.

For this ticket, rewrite the QA client to be more consistent with our other API clients. It should retain the existing functionality, as well as adding the functionality described in #3189.

@jcoyne
Copy link
Contributor

jcoyne commented Oct 29, 2021

@justinlittman You aren't supposed to read or make changes to a generate client. You just need to update the spec and regenerate. Why don't we just maintain the openapi spec?

@justinlittman
Copy link
Contributor Author

image

@justinlittman
Copy link
Contributor Author

We could maintain our own local openapi, but since we don't maintain QA this is fragile.

It would be far easier to maintain a simple client.

@jcoyne
Copy link
Contributor

jcoyne commented Oct 29, 2021

@justinlittman There is an swagger doc on that site here: https://lookup.ld4l.org/qa/apidoc/apidoc.json Is there a reason we can't ask Lynette to update it with her changes? They are funded on this grant too, right? I don't want this API that is very important to our project to be subject to arbitrary changes that are not validatable and documented.

@justinlittman
Copy link
Contributor Author

Yes, we can (and will) recommend that that swagger doc be maintained.

@justinlittman
Copy link
Contributor Author

See LD4P/qa_server#464

@mjgiarlo mjgiarlo self-assigned this Nov 8, 2021
mjgiarlo added a commit that referenced this issue Nov 8, 2021
Fixes #3262

This commit replaces the swagger-based QA client. Why?

1. the API specification is incomplete;
2. it's not clear there is a plan in place to keep the specification up to date; and
3. we only use a fraction of the QA API in the Sinopia editor

In short, it's to pay down technical debt. This commit otherwise retains the same interface for interacting with the QA API, returning a promise that resolves to the HTTP response.
@justinlittman justinlittman reopened this Nov 12, 2021
@mjgiarlo
Copy link
Member

@justinlittman 👀

justinlittman pushed a commit that referenced this issue Nov 17, 2021
Fixes #3262

This commit replaces the swagger-based QA client. Why?

1. the API specification is incomplete;
2. it's not clear there is a plan in place to keep the specification up to date; and
3. we only use a fraction of the QA API in the Sinopia editor

In short, it's to pay down technical debt. This commit otherwise retains the same interface for interacting with the QA API, returning a promise that resolves to the HTTP response.
justinlittman pushed a commit that referenced this issue Nov 17, 2021
Fixes #3262

This commit replaces the swagger-based QA client. Why?

1. the API specification is incomplete;
2. it's not clear there is a plan in place to keep the specification up to date; and
3. we only use a fraction of the QA API in the Sinopia editor

In short, it's to pay down technical debt. This commit otherwise retains the same interface for interacting with the QA API, returning a promise that resolves to the HTTP response.
justinlittman pushed a commit that referenced this issue Nov 18, 2021
Fixes #3262

This commit replaces the swagger-based QA client. Why?

1. the API specification is incomplete;
2. it's not clear there is a plan in place to keep the specification up to date; and
3. we only use a fraction of the QA API in the Sinopia editor

In short, it's to pay down technical debt. This commit otherwise retains the same interface for interacting with the QA API, returning a promise that resolves to the HTTP response.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants