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

customize-schemas.php is not idempotent #22

Open
dpash opened this issue Feb 19, 2024 · 3 comments
Open

customize-schemas.php is not idempotent #22

dpash opened this issue Feb 19, 2024 · 3 comments

Comments

@dpash
Copy link

dpash commented Feb 19, 2024

Running customize-schemas.php multiple times results in further modifications of the schemas.

There's three issues:

  1. Security information is stripped away, as it doesn't take existing data into account.
  2. 'x-expose-headers' is added where as previously it's not included. This seems to be a bug due to it being stripped out when first run.
  3. Additions are added multiple times (which results in invalid json due to duplicate parameters in orders.json)

I have fixes for the first two and I'll create a PR for these. I still need to figure out how to resolve the addition issue.

An alternative approach is to download the original schemas to a different directory and then have the customize-schemas.php write out the schemas to new files.

@jlevers
Copy link
Contributor

jlevers commented Feb 22, 2024

Hey @dpash -- thanks for the well-thought-out issue.

Totally agree that this is a problem. I've reworked the schema download/customize/generate pipeline in the (still WIP) v6 of the jlevers/selling-partner-api repo to be a) idempotent and b) built using Symfony commands to make it simpler to work on. I'll revisit this repo once that update's done, and make similar updates here.

Appreciate the offer of a PR -- for the sake of simplicity, I'd like to keep the pipeline for this repo as similar as possible to the pipeline in that v6 branch linked above. If you'd be willing to check that out and see if it's easy to wrap your fixes into a similar structure, that would be much appreciated.

@dpash
Copy link
Author

dpash commented Jul 23, 2024

Hey, have you done any work on migrating this library to use saloon, or should I look into doing so? Seems Walmart have considerably updated their JSON schema since this library was last built and it no longer builds. Seems pointless trying to fix up the old build system if you're planning on migrating away.

@jlevers
Copy link
Contributor

jlevers commented Jul 31, 2024

I haven't had a chance to get started on migrating to Saloon, no, although I would like to do so eventually. If you're feeling inspired and want to get started on the revamp, please feel free! Happy to review PRs. Keeping the structure as similar as possible to jlevers/selling-partner-api will make maintenance easier. I think you're pretty familiar with that repo, but I'm happy to provide guidance if you need it.

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

No branches or pull requests

2 participants