apigentools icon indicating copy to clipboard operation
apigentools copied to clipboard
verified publisher icon DataDog

Keep order of params when generating full spec

Open d-mo opened this issue 5 years ago • 8 comments

What does this PR do?

This PR utilizes yamlloader.ordereddict when loading the input yaml files. This way the order of the attributes does not change when generating the full specification.

Review checklist (to be filled by reviewers)

  • [ ] Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • [ ] PR title must be written as a CHANGELOG entry (see why)
  • [ ] Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • [ ] PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • [ ] PR should not have do-not-merge/ label attached.
  • [ ] If Applicable, issue must have kind/ and severity/ labels attached at least.

d-mo avatar Sep 27 '20 13:09 d-mo

@jirikuncar Done

d-mo avatar Oct 20 '20 11:10 d-mo

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity.

github-actions[bot] avatar Nov 20 '20 00:11 github-actions[bot]

Hi @jirikuncar

I rebased the PR and @ckarageorgkaneen extended it to include the dicts under the components section.

Are you interested in merging it? Is there anything else missing?

d-mo avatar Sep 13 '21 11:09 d-mo

@jirikuncar Thank you for the suggestions! I applied both.

d-mo avatar Sep 16 '21 12:09 d-mo

@d-mo I have done some testing and it is way slower when merging our large OpenAPI specifications.

# master
$ time apigentools merge

real    0m1.612s
user    0m1.536s
sys     0m0.067s

# pr/204
$ time apigentools merge

real    0m10.816s
user    0m10.577s
sys     0m0.107s

Can we find a way that we could still use the CSafeDumper and CSafeLoader together with the OrderedDict?

jirikuncar avatar Sep 23 '21 15:09 jirikuncar

@jirikuncar Can you test again? I was able to use CSafeLoader & CSafeDumper after switching to yamlloader.

d-mo avatar Sep 25 '21 16:09 d-mo

@d-mo it seems fine now. I don't see any major performance issue on large files.

jirikuncar avatar Sep 27 '21 10:09 jirikuncar

Hi @jirikuncar.

I fixed the conflicts that came up. Is there any interest in merging this PR at some point?

We'd love to use the official apigentools instead of maintaining our own fork. I think the changes might be useful to others as well.

Thanks

d-mo avatar May 10 '22 12:05 d-mo