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

Revert "Fix --sync missing some recipes" #123

Merged
merged 1 commit into from
Sep 8, 2024
Merged

Conversation

kylewlacy
Copy link
Member

Reverts #122

Okay, it finally hit me what was happening...

So the way it was previously implemented --sync would find all top-level recipes for a build, then would sync them plus any recipes that they referenced by hash. #122 changed it so it would find all internal recipes, either by hash or nested directly. The PR changed it so these internal recipes would be synced too

I think it would be a valid decision to sync these internal recipes, but for the purposes of --sync, I think it's the wrong choice. Namely, it just pollutes the registry with some recipes that should never get directly downloaded (clients only download the recipes referenced by hash, they would never have a need to download a recipe embedded directly)

As for the issue with eza, I finally realized that it was just because some of the bakes I had locally were non canonical (meaning that what was stored in my local DB was different than the registry), which meant that later recipes diverged enough that I got a cache miss.

Basically, all that is to say is that everything was already working as expected prior to #112, although the end result was unintuitive. Upon reflection, I felt that the earlier behavior was more in line with how syncing should work

@kylewlacy
Copy link
Member Author

As part of #112, I ended up re-running the latest CI build for brioche-packages (CI #278). This meant the extra recipes uploaded from #112 already ended up in the registry

As part of the revert, I rolled back the database to 2024-09-07 at 12:00am. Since the brioche-packages repo hadn't been updated in a while, this only impacted the newly inserted recipes, so the effects of #112 have been undone in the registry too

@kylewlacy kylewlacy merged commit 0eaeeb2 into main Sep 8, 2024
5 checks passed
@kylewlacy kylewlacy deleted the revert-122-missing-sync branch September 8, 2024 02: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.

1 participant