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

fix: ensure scalar examples are consistent #976

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jedevc
Copy link

@jedevc jedevc commented Aug 7, 2024

Heya! While working with spectaql, I noticed that example outputs produce random results - this can be an issue, since it means that multiple runs of spectaql with the same input can very easily produce different output.

Ideally, this shouldn't be the case, and all output should be as consistent and reproducible as possible.

Maybe this isn't the right way to go about this! 🎉 And there's a neater way to do it, maybe a setting that allows for getting reproducible output by inspecting the environment/etc - happy to update the PR to do something else, if this approach isn't desirable.

These example outputs produce random results - this is an issue, since
it means that multiple runs of spectaql with the same input can very
easily produce different output.

Ideally, this shouldn't be the case, and all output should be as
consistent and reproducible as possible.

Signed-off-by: Justin Chadwell <[email protected]>
@jedevc jedevc changed the title fix: remove random scalar examples fix: ensure scalar examples are consistent Aug 7, 2024
@newhouse newhouse self-requested a review August 9, 2024 20:22
@newhouse
Copy link
Collaborator

newhouse commented Aug 9, 2024

HI there @jedevc and thanks for this!

I'm going to have to think about this a bit more as I think there's maybe a different approach. This idea/problem has come up before and I think the solution is more complicated than these changes right here.

@jedevc
Copy link
Author

jedevc commented Aug 12, 2024

Thanks @newhouse! Feel free to let me know if you have ideas for a different approach, I'm happy to dive in a bit more, I'm currently (ab)using a fork so we can get this setup working, but I'd much rather use upstream as much as possible! 🎉

Maybe an approach where we could take the hash of some canonicalized version of the graphql schema, and then use examples[hash(<schema hash> + <example name>) % len(examples)]? Then you could still have the random examples, but they're now deterministic based on the hash of the schema? (though that is still a bit weird that changing unrelated parts of the schema causes changes in the examples for other parts of the schema).

@marknuzz
Copy link

I was disappointed to see the large diffs created by the generator, as it means committing the output to a repository becomes complicated and any 'real' diffs become lost in the noise, among other complications. Would love to see an update from the maintainer.

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.

3 participants