Skip to content

Project Synchronization #12

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

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Conversation

kewisch
Copy link
Member

@kewisch kewisch commented Feb 20, 2025

Making a start on getting this off my chest and back into the core repo. This is a pretty major change in terms of code structure and configuring, but shouldn't (tm) break any existing workflows.

It is probably easier to review the code in its source rather than looking at the diff https://github.com/kewisch/thunderbird-notion-scripts/tree/gh_project

I have the GitHub Actions automation running in my repo on the main branch. The cron job is set to every 5 minutes, but in reality runs about every 15 minutes. For my purposes that is (currently) good enough, but might not work for everyone.

I have run the synchronizer with the original services code in my test repo and personal notion instance and it seems to work, but it would probably be a good idea to run it on something that is closer to prod to verify it still works as expected. A notable change is that rather than sleeping between requests I wait until the 429 and then sleep for the indicated number of seconds. I haven't actually triggered rate limiting, so I'm unsure if that works.

I have not tested the bugzilla code at all, are we actively using that? It seemed a bit more experimental.

@kewisch
Copy link
Member Author

kewisch commented Feb 20, 2025

I don't have complete trust in the notion to markdown and markdown to notion python packages, we should probably pin those if rye allows for that and then review before upgrade. I wasn't able to find a well maintained notion<>markdown package that converts both ways and allows to easily separate out the conversion process.

@Sancus
Copy link
Member

Sancus commented Feb 20, 2025

I haven't gone through everything yet, but one thing I don't really like structurally about this is that it merges the separate scripts together and runs them sequentially. That's:

  • Bad for performance(all syncs need to finish running before any other can run again)
  • Doesn't scale(what happens if we have 10 syncs? All 10 should be able to run parallel)
  • Means that all syncs are dependent. If there is an uncaptured exception anywhere, all syncs fail. You also can't easily run them at different frequencies, or anything like that.

@kewisch
Copy link
Member Author

kewisch commented Feb 21, 2025

I added a commit that uses GitHub actions matrix strategy to run each sync in parallel that should alleviate above concerns.

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