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

Generated API Client #12071

Merged
merged 112 commits into from
May 23, 2022
Merged

Generated API Client #12071

merged 112 commits into from
May 23, 2022

Conversation

krishnaglick
Copy link
Contributor

@krishnaglick krishnaglick commented Apr 15, 2022

Our service layer is now auto-generated from the OpenApi spec.

Closes #10181

@github-actions github-actions bot added area/frontend area/platform issues related to the platform labels Apr 15, 2022
@krishnaglick krishnaglick marked this pull request as ready for review April 15, 2022 17:15
@krishnaglick krishnaglick requested a review from a team as a code owner April 15, 2022 17:15
@dizel852
Copy link
Contributor

Extra "/" issue during creation of GitHub source
Screenshot at May 16 17-31-34

Tim Roes and others added 2 commits May 17, 2022 14:41
Co-authored-by: Vladimir <volodymyr.s.petrov@globallogic.com>
Co-authored-by: Vladimir <volodymyr.s.petrov@globallogic.com>
@dizel852
Copy link
Contributor

Looks like one more issue:
I noticed that on the Sync History table we have short useful info (master branch):

Screenshot at May 18 23-51-13

But on this branch, they partially disappeared - Job type is gone, but "how many record" moved to JobLogs component:

Screenshot at May 18 23-51-24

image

Is it desired behavior? 🤔

@timroes
Copy link
Contributor

timroes commented May 19, 2022

Is it desired behavior? thinking

This is def not desired behavior and will look to get this fixed

@timroes
Copy link
Contributor

timroes commented May 19, 2022

Another bug: the logs view with multiple attempts should not be working atm, since the current code only pulls of the latest logs always. Going to look into that as well.

Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Tested the whole app thoroughly with this PR active. OSS as well as in Cloud. Looked at the code overall and the specific core files like apiOverride (have not reviewed the type only changes in every single file on detail). This looks as safe as we can get it before merging for me.

@dizel852
Copy link
Contributor

I'm not sure if this issue relates to changes in this PR, but after the creation of a new connection, the sync job starts automatically(does it should?).
If we will try to cancel it, it will launch the sync again. So we are in the loop.
Was testing with different sources and dest - the same.
https://www.loom.com/share/8dae4109f7ff4e37ad8f41aeb1888d4b

Double-checked on stage env - didn't reproduce.
Wasn't unable to check this case on prod since I run out of credits

@timroes
Copy link
Contributor

timroes commented May 20, 2022

@dizel852 The start behavior is expected, that we immediately start a new job when creating a connection (and I verified it against prod). The repeating when cancelling the initial sync sounds weird (but also present on prod). I'll check with the other platform teams, because that does indeed look suspicious (but since both are present on prod, not a problem with this PR).

@dizel852
Copy link
Contributor

@timroes ok, then.
No more issues were found.
Was testing locally (OSS)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate TypeScript code from OpenAPI spec
6 participants