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.

Sign up for GitHub

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

Jump to bottom

perf(openapi): upload improvements #518

Merged

kanadgupta merged 13 commits into main from kanad/rm-4367-update-rdme-to-support-new-oas-upload

Jun 23, 2022

Member

kanadgupta commented Jun 17, 2022 •

edited

Loading

🚥 Fix RM-4367

(hopefully) fixes #499

🧰 Changes

Behind-the-scenes improvements to openapi command to support new two-part spec upload process 🚀
Updated test coverage to reflect these changes¹ 📈
Introduced ora into the mix so users can have better visibility into the validation/upload process 🧭

Previous TODOs (archived)

Function for uploading spec to registry 🚀
Loading spinners for validation ✅
Updates to openapi command to support new process ✨
Base config for ora loading spinners 🧭
Tests 🧪

🧬 QA & Testing

Do tests pass? You can also test this command with a project of your own by checking out this branch and running:

bin/rdme openapi __tests__/__fixtures__/ref-oas/petstore.json --key=<api key>

I did a lot of test coverage/cleanup work as part of these changes, but that blew up the diff and scope of this PR so any changes unrelated to this new openapi process can be found in #520. ↩

kanadgupta added 4 commits

June 17, 2022 15:11


          chore(deps): install ora

9ee396c


          chore: add ora to dependabot and reformat

97e7678


          feat: add spinner for file validation

0ad8d64


          feat: initial registry upload function

kanadgupta added enhancement performance labels

kanadgupta added 7 commits

June 21, 2022 18:36


          refactor: move ora opts into shared function

813b158


          refactor: use shared ora opts in new function also

574d2a9


          feat: first pass at rewriting openapi command to support new flow

f53b758

- also i realphabetized the imports


          chore: making the spinners spin

e58a092


          test: refactor existing tests to work with new flow

8a748db


          test: add coverage for if registry endpoint fails

5ecce6d


          chore: test coverage in ora opts

56489df

github-advanced-security bot found potential problems

View reviewed changes

github-advanced-security bot left a comment

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.


          chore: spinner language

d86369e

github-advanced-security bot found potential problems

View reviewed changes

github-advanced-security bot left a comment

Found 11 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security bot found potential problems

View reviewed changes

__tests__/cmds/openapi.test.js

@@ @@ -98,17 +104,21 @@ @@
                   it('should discover and upload an API definition if none is provided', async () => {
                     promptHandler.createOasPrompt.mockResolvedValue({ option: 'create' });
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -139,8 +149,12 @@ @@
                     ['OpenAPI 3.1', 'json', '3.1', 'OpenAPI'],
                     ['OpenAPI 3.1', 'yaml', '3.1', 'OpenAPI'],
                   ])('should support updating a %s definition (format: %s)', async (_, format, specVersion, type) => {
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -159,8 +173,12 @@ @@
                   });
                   it('should return warning if providing `id` and `version`', async () => {
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -217,17 +235,21 @@ @@
                       newVersion: '1.0.1',
                     });
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -241,19 +263,22 @@ @@
                 it('should bundle and upload the expected content', async () => {
                   let requestBody = null;
+                  const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -270,19 +295,22 @@ @@
                 it('should use specified working directory and upload the expected content', async () => {
                   let requestBody = null;
+                  const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -367,14 +395,18 @@ @@
                       help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
                     };
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -399,14 +457,18 @@ @@
                       help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
                     };
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -419,14 +481,18 @@ @@
                   });
                   it('should error if API errors (generic upload error)', async () => {
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

__tests__/cmds/openapi.test.js

@@ @@ -439,14 +505,18 @@ @@
                   });
                   it('should error if API errors (request timeout)', async () => {
+                    const registryUUID = getRandomRegistryId();

Check notice

Code scanning

Insecure randomness

Cryptographically insecure [random value](1) in a security context.

Member Author

kanadgupta commented Jun 23, 2022 •

edited

Loading

codeql blowing up the PR with comments now (that i'm unable to delete)... (shakes fist)

kanadgupta marked this pull request as ready for review

June 23, 2022 02:24

kanadgupta requested review from erunion, a team, Dashron and domharrington and removed request for a team

June 23, 2022 02:26

erunion approved these changes

View reviewed changes

Member

erunion left a comment

everything here looks pretty solid!


          Merge branch 'main' into kanad/rm-4367-update-rdme-to-support-new-oas…

f4b259e

…-upload

kanadgupta merged commit e1aeac0 into main

kanadgupta deleted the kanad/rm-4367-update-rdme-to-support-new-oas-upload branch

June 23, 2022 17:02

kanadgupta mentioned this pull request

test(openapi/validate): cleanup 🧹 #520

Merged

3 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement performance