airbnb/knowledge-repo

Bug in support for non-master default branches for repos

e-m-albright opened this issue · 2 comments

e-m-albright commented

Hello! My team has made the decision to move our tools away from using master, instead opting for main as the new default branch pattern.

When I run knowledge_repo add ... I get an error message that stems from the following code flow in updating the self.config of one of the repo objects.

  1. Set the default published branch to "master"
  2. Read the config of the repository for updated values using the git_read method
  3. Break, as the git reader requires a valid published_branch value and is currently looking for master before it can set it to the repo's config override here in the reader
  4. The issue is raised as a git.GitError and suppressed, where I'm seeing a different issue later as the repo object is None

Thanks for any support here!

I'll try my hand at a pull request as I can

bulam commented

Hi Evan, Thanks for filing this issue. I'm a bit short on bandwidth at the moment, so might not be able to help debug, but if you do get to submit a PR let's definitely merge it! :)

e-m-albright commented

Hey! I tried a few things and landed on this PR #571

I also considered that for your for_uri factory method you might want to pass in args entirely as it seems like the knowledge_branch arg could be useful here to the constructor in replacing the master reference, but it felt like it wasn't a terribly clean fix.