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

[Refactor] Remove TinyDB #468

Merged
merged 8 commits into from
Aug 28, 2023
Merged

Conversation

frankharkins
Copy link
Member

Summary

Removes TinyDB in favour of reading/writing from TOML files directly.

@frankharkins frankharkins force-pushed the fh-refactor-tinydb branch 5 times, most recently from 9a1b348 to 5b60477 Compare July 31, 2023 12:54
@frankharkins frankharkins marked this pull request as ready for review July 31, 2023 13:00
Copy link
Collaborator

@mickahell mickahell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 🚀 :)

Did you run some Actions to tests :

  • submission
  • update badges/stars
  • Batch | merge check files and push to db

Just to be sure for any inputs/outputs ;)

@frankharkins
Copy link
Member Author

I believe it's working:

@mickahell
Copy link
Collaborator

mickahell commented Aug 4, 2023

I believe it's working:

Cool :)
Seems working for badges/stars.

The commit of the PR generated by the submission wf : frankharkins@426423b should have only one file modified, not every member :o
Look at this step : https://github.com/frankharkins/ecosystem/actions/runs/5763659033/job/15626180458#step:12:58
Maybe it's just because your main branch wasn't sync we the base repo

@frankharkins
Copy link
Member Author

frankharkins commented Aug 5, 2023

It's very strange; it seems the commit is changing all those files, but I can't reproduce it locally when I run python manager.py add_repo_2db ...😕

EDIT: After a pip install ... I can now reproduce. I think there's two problems:

  1. There was a bug in which we overwrite timestamps when they already exist (fixed in dcd06bd)
  2. I think the data we're passing to toml.dump is now in a slightly different order, which means each file has changed a bit. In commit 7c99dae I re-order the entire library and it seems to sort the problem out.

This seems to fix the problem (see frankharkins#7).

@mickahell
Copy link
Collaborator

mickahell commented Aug 5, 2023

Indeed seems to be working as usual now :)

About the toml reordering, if you run the recompile endpoint, you shouldn't have any change, maybe you can test with that.

Copy link
Collaborator

@mickahell mickahell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to reverse the changes applied in .toml files

@frankharkins frankharkins force-pushed the fh-refactor-tinydb branch 2 times, most recently from 38d0f25 to ee8f97c Compare August 24, 2023 13:03
@frankharkins
Copy link
Member Author

Tested members.json locally (see 38d0f25 – not on this branch) and worked correctly.

@frankharkins frankharkins merged commit d6df6e9 into Qiskit:main Aug 28, 2023
4 checks passed
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