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

Replace swagger-based QA client with fetch calls #3312

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

mjgiarlo
Copy link
Member

@mjgiarlo mjgiarlo commented Nov 8, 2021

Why was this change made?

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.

How was this change tested?

CI

Which documentation and/or configurations were updated?

None

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.
@mjgiarlo mjgiarlo marked this pull request as draft November 8, 2021 21:37
@mjgiarlo mjgiarlo marked this pull request as ready for review November 8, 2021 22:00
@mjgiarlo mjgiarlo changed the title Replace swagger-based QA client with simple fetch calls Replace swagger-based QA client with fetch calls Nov 8, 2021
@jcoyne
Copy link
Contributor

jcoyne commented Nov 8, 2021

I'm quite against the lack of a shared and validatable contract about this API that is maintained by two entities. I've recently worked with Lynette to get the API matching the swagger spec they are publishing. I think we should continue to use it.

@mjgiarlo
Copy link
Member Author

mjgiarlo commented Nov 8, 2021

@jcoyne 💬

I'm quite against the lack of a shared and validatable contract about this API that is maintained by two entities. I've recently worked with Lynette to get the API matching the swagger spec they are publishing. I think we should continue to use it.

After chatting with @justinlittman this morning, it seems the fetch-based approach is preferred at this time. Maybe a stand-up discussion is warranted.

@justinlittman justinlittman merged commit 414f263 into main Nov 9, 2021
@justinlittman justinlittman deleted the rewrite-qa-client#3262 branch November 9, 2021 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite QA client
3 participants