Skip to content

[MAINT] Fixup github_repository_file#3175

Merged
stevehipwell merged 19 commits intointegrations:mainfrom
F-Secure-web:update-repository_file-import-docs
Feb 11, 2026
Merged

[MAINT] Fixup github_repository_file#3175
stevehipwell merged 19 commits intointegrations:mainfrom
F-Secure-web:update-repository_file-import-docs

Conversation

@deiga
Copy link
Copy Markdown
Collaborator

@deiga deiga commented Feb 9, 2026 •
edited
Loading

Resolves #3161

Before the change?

  • There was no test for importing github_repository_file
  • Import format: repo/file or repo/file:branch; default branch was imported without an explicit branch segment.
  • Branch field: branch was optional and not Computed; state could omit it for default-branch resources.
  • State: No migration for old ID formats; repository_id was not part of the resource’s state.
  • autocreate_branch: Used in Read, Update, and Delete; no deprecation notice.

After the change?

  • We have tests for importing github_repository_file
  • Adds Computed: true to branch
  • Adds computed repository_id field and enables repo renaming without resource recreation
  • Computes branch on create or import if not specified
  • Changes ID schema from <repo>/<file> to <repo>:<file>:<branch> to enable safer usage of multiple files from same repo but different branches
  • Adds State migration for computing branch, repository_id and new ID format
  • Import format: repo:file:branch (colon-separated); default branch uses repo:file: (trailing colon). Matches the internal ID format.
  • autocreate_branch: Deprecated and no longer used in Read, Update, or Delete.

Pull request checklist

  • Schema migrations have been created if needed (example)
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 9, 2026

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@deiga deiga requested review from nickfloyd and stevehipwell and removed request for nickfloyd February 9, 2026 17:16
@deiga deiga marked this pull request as ready for review February 9, 2026 17:16
@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 9, 2026

image

deiga added 3 commits February 9, 2026 19:18
@deiga
Improve docs of importing github_repository_file
c634707 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
Add missing test for import
3965ecb 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
Ensure we only set branch on import if it's not the default branch
aef6dcf 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga deiga force-pushed the update-repository_file-import-docs branch from 1337fdb to aef6dcf Compare February 9, 2026 17:18
nickfloyd
nickfloyd previously approved these changes Feb 9, 2026
View reviewed changes
stevehipwell
stevehipwell requested changes Feb 9, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

I missed the fact that the branch is optional, I think this should be required in the next major version. As it's optional then the import ID should allow it to be not set.

@nickfloyd nickfloyd self-requested a review February 9, 2026 17:22
@stevehipwell
Copy link
Copy Markdown
Collaborator

stevehipwell commented Feb 9, 2026

So what I'm saying is the following changes are needed to fix this resource.

  • Add Computed: true to branch
  • Compute branch on create if not specified
  • Support importing with ID missing branch (compute on import only if missing)
  • Add migration to fix defect with ID not containing branch and missing branch computed value

deiga added 3 commits February 9, 2026 20:46
@deiga
Update branch to Computed field
6b2938f 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
Add state migration for missing branch field
76e4f34 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
Add ID migration in the same bunch
db068df 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga deiga requested a review from stevehipwell February 9, 2026 20:12
@deiga
Fix ID parsing
3c2e590 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
stevehipwell
stevehipwell requested changes Feb 9, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

We can simplify the logic by always requiring a 2 part ID, with the second part blank for the default ID; e.g. my-repo/my-file.txt:.

Longer term the ID should be in the form <repo>:<file>:<branch>, but that's a problem for another day.

@deiga
fixup! Fix ID parsing
e75070b 
Always use 2 part ID for import. Use buildID for resource ID
@deiga deiga requested a review from stevehipwell February 10, 2026 09:32
@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 10, 2026

@stevehipwell Done. But I wonder if I should just refactor the import ID now as well? Sure, it's technically a breaking change, but imports are "one-off" usually

@stevehipwell
Copy link
Copy Markdown
Collaborator

stevehipwell commented Feb 10, 2026

@stevehipwell Done. But I wonder if I should just refactor the import ID now as well? Sure, it's technically a breaking change, but imports are "one-off" usually

@deiga I think this makes sense. @nickfloyd thoughts?

We should also add repository_id to the schema as part of the migration, add the diffRepository, and remove ForceNew from repository; this will stop files being destroyed if the repo name changes.

deiga added 3 commits February 10, 2026 20:53
@deiga
fixup! fixup! Fix ID parsing
581f684 
Use 3 part ID for import as well
@deiga
fixup! fixup! fixup! Add computed repository_id field to resource
9b9f8b8 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
fixup! fixup! fixup! fixup! Add migration of the repository_id field.
46162f6 
This required changing all tests to need a mock of the GH API

Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 10, 2026

@stevehipwell Addressed your comments. And re-introduced the API Mock pattern here, since it's the only way to verify the migration.

Would you still prefer that we discuss before we can add this pattern?

@stevehipwell
Copy link
Copy Markdown
Collaborator

stevehipwell commented Feb 10, 2026

Would you still prefer that we discuss before we can add this pattern?

Yes, I don't think this is the time to add this; but I think it's something we need to explore further.

@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 10, 2026

@stevehipwell Done

stevehipwell
stevehipwell requested changes Feb 10, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

I've added inline comments, but you'll also need to add an ID set to update as the ID needs to be updated if the repo name has changed.

@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 10, 2026

@stevehipwell

you'll also need to add an ID set to update as the ID needs to be updated if the repo name has changed.

Doesn't ForceNew run Create, so that shouldn't be needed in Update?

@stevehipwell
Copy link
Copy Markdown
Collaborator

stevehipwell commented Feb 10, 2026

Doesn't ForceNew run Create, so that shouldn't be needed in Update?

The diff function supports changing the repo name without forcing new, so we have to set the ID in update.

@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 10, 2026

@stevehipwell If the repository_id changes, then we set ForceNew on repository. Which triggers Create instead if Update, or do I have that mixed up?
Is there any case where repository_id would change and need to be set in Update?

@deiga
fixup! Address review comments
b07ed84 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga deiga requested a review from stevehipwell February 10, 2026 21:38
@stevehipwell
Copy link
Copy Markdown
Collaborator

stevehipwell commented Feb 10, 2026

@stevehipwell If the repository_id changes, then we set ForceNew on repository. Which triggers Create instead if Update, or do I have that mixed up?
Is there any case where repository_id would change and need to be set in Update?

The resource ID uses repo name so we need to call d.SetId(), not update the repository_id as that is static.

stevehipwell
stevehipwell requested changes Feb 10, 2026
View reviewed changes
@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 11, 2026

@stevehipwell Doh! 🤦
Sorry, I kept misunderstanding your point

deiga added 3 commits February 11, 2026 05:45
@deiga
fixup! Address review comments
5e43cb5 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
fixup! Remove usage of autocreate_branch in Read, Update and `Del…
0d8ebfe 
…ete`

Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
Mark autocreate_branch as deprecated
d2f19df 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga deiga requested a review from stevehipwell February 11, 2026 04:00
@deiga
Copy link
Copy Markdown
Collaborator Author

deiga commented Feb 11, 2026

@stevehipwell I went and deprecated the whole autocreate_branch stuff, let me know if I went too far :D

And, could you update the title of this PR to something like "resource_github_repository_file: fix import format and add state migrations"

stevehipwell
stevehipwell reviewed Feb 11, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

This is looking good @deiga, just a couple of things to look at.

deiga added 2 commits February 11, 2026 15:48
@deiga
fixup! Address review comments
aa0a114 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga
fixup! Change to use UpdateFile in Update
1fe800a 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@deiga deiga requested a review from stevehipwell February 11, 2026 14:24
stevehipwell
stevehipwell reviewed Feb 11, 2026
View reviewed changes
stevehipwell
stevehipwell reviewed Feb 11, 2026
View reviewed changes
stevehipwell
stevehipwell requested changes Feb 11, 2026
View reviewed changes
@deiga
fixup! Update docs with missing fields
e2cb7f4 
Signed-off-by: Timo Sand <timo.sand@f-secure.com>
@stevehipwell stevehipwell changed the title [MAINT] Improve docs of github_repository_file [MAINT] Fixup github_repository_file Feb 11, 2026
@stevehipwell stevehipwell added this to the v6.11.1 Patch milestone Feb 11, 2026
@deiga deiga requested a review from stevehipwell February 11, 2026 18:15
stevehipwell
stevehipwell approved these changes Feb 11, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

LGTM

@stevehipwell stevehipwell merged commit 256cdd8 into integrations:main Feb 11, 2026
7 checks passed
@deiga deiga deleted the update-repository_file-import-docs branch February 11, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickfloyd nickfloyd nickfloyd approved these changes

@stevehipwell stevehipwell stevehipwell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v6.11.1 Patch

Development

Successfully merging this pull request may close these issues.

[BUG]: Unable to import a github_repository_file from default branch with provider 6.11.0

3 participants

@deiga @stevehipwell @nickfloyd