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: Add fields to generated files in deterministic order #45

Merged

Conversation

mklaber
Copy link
Contributor

@mklaber mklaber commented Jun 22, 2021

Description

Sometimes the QB API likes to return fields in a different order. The only way to ensure that any two runs of qbc run model-generate produce the same output, we need to sort the fields each time.

This PR:

  • Modifies the get_fields_for_table_aaaaaa.json mock data to illustrate this phenomenon
  • Sorts fields by field ID before adding them to a file
  • Tests this ordering

Checklist:

  • Submitting to dev branch
  • pytest tests passing
  • Includes tests to cover the changes
  • flake8 passing
  • Relevant documentation added

Sometimes the QB API likes to return fields in a different order. In order to make sure that any two runs of `qbc run model-generate` produce the same output, we need to sort the fields each time.
@mklaber
Copy link
Contributor Author

mklaber commented Jun 22, 2021

@tkutcher If you want to see an example of where I observed this behaviour feel free to email me at klaber at gmail. I'd rather not post it publicly.

@tkutcher
Copy link
Owner

Never realized this. I'm good with this - will take your word for it! The sort certainly won't hurt anything. I'll get it in there soon.

@tkutcher tkutcher merged commit 9019941 into tkutcher:dev Jun 30, 2021
tkutcher added a commit that referenced this pull request Jun 30, 2021
@tkutcher tkutcher added this to the v0.4 milestone Jun 30, 2021
@tkutcher tkutcher added the type:bug 🐛 Something isn't working label Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants