Skip to content

s/Hash/BTree/g in codegen to make the output deterministic #430

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

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

bolinfest
Copy link
Contributor

We were seeing cases where #[derive(GraphqlQuery)] was not generating
consistent output. This appeared to be due to the use of HashMap and
HashSet, whose iteration order is not guaranteed to be consistent.
Switching to BTreeMap and BTreeSet appears to fix the issue.

@bolinfest
Copy link
Contributor Author

/cc @tomhoule @LegNeato it looks like an approval is required to run the workflow?

@LegNeato
Copy link
Member

👋. Looks good, running CI. Also, hit me up and let's chat, been a long time!

We were seeing cases where `#[derive(GraphqlQuery)]` was not generating
consistent output. This appeared to be due to the use of `HashMap` and
`HashSet`, whose iteration order is not guaranteed to be consistent.
Switching to `BTreeMap` and `BTreeSet` appears to fix the issue.
@bolinfest bolinfest force-pushed the deterministic-derive branch from 69ca536 to 1dc6d20 Compare August 11, 2022 06:04
@bolinfest
Copy link
Contributor Author

@LegNeato indeed, we are long overdue!

I added a commit to the bottom of the stack in this PR to fix the clippy issue caught by CI.

There is also a Prettier issue, but I think the output is also complaining about a bunch of stuff that appears unrelated to my PR.

@bolinfest
Copy link
Contributor Author

Specifically, this runs with an exit code of 1:

npx prettier --debug-check -l './graphql_client/**/*.graphql'

but based on the output, I'm not clear what it is complaining about.

@bolinfest
Copy link
Contributor Author

FWIW, as best I can tell, the cargo test failure was a temporary network error when fetching a dependency?

Updating crates.io index
warning: spurious network error (2 tries remaining): SecureTransport error: connection closed via error; class=Net (12)
warning: spurious network error (1 tries remaining): SecureTransport error: connection closed via error; class=Net (12)
error: failed to get `reqwest` as a dependency of package `graphql_client v0.11.0 (/Users/runner/work/graphql-client/graphql-client/graphql_client)`

Caused by:
  failed to load source for dependency `reqwest`

Caused by:
  Unable to update registry `crates-io`

Caused by:
  failed to fetch `https://github.com/rust-lang/crates.io-index`

Caused by:
  network failure seems to have happened
  if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
  https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

Caused by:
  SecureTransport error: connection closed via error; class=Net ([12](https://github.com/graphql-rust/graphql-client/runs/7782827120#step:4:13))

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.

2 participants