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

feat: GithubAppInstallation model #236

Merged
merged 2 commits into from
Jan 29, 2024
Merged

Conversation

giovanni-guidini
Copy link
Contributor

These changes port codecov/codecov-api#346 to the worker so we can use them here as well.

context: codecov/engineering-team#970

@codecov-staging
Copy link

codecov-staging bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #236   +/-   ##
=======================================
  Coverage   98.13%   98.14%           
=======================================
  Files         375      375           
  Lines       30667    30721   +54     
=======================================
+ Hits        30096    30150   +54     
  Misses        571      571           
Flag Coverage Δ
integration 98.14% <100.00%> (+<0.01%) ⬆️
latest-uploader-overall 98.14% <100.00%> (+<0.01%) ⬆️
unit 98.14% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.21% <100.00%> (+<0.01%) ⬆️
OutsideTasks 97.92% <100.00%> (+<0.01%) ⬆️
Files Coverage Δ
database/models/core.py 97.28% <100.00%> (+0.18%) ⬆️
database/tests/unit/test_models.py 100.00% <100.00%> (ø)

@codecov-qa
Copy link

codecov-qa bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c248c6f) 98.13% compared to head (32a13ed) 98.14%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #236   +/-   ##
=======================================
  Coverage   98.13%   98.14%           
=======================================
  Files         375      375           
  Lines       30667    30721   +54     
=======================================
+ Hits        30096    30150   +54     
  Misses        571      571           
Flag Coverage Δ
integration 98.14% <100.00%> (+<0.01%) ⬆️
latest-uploader-overall 98.14% <100.00%> (+<0.01%) ⬆️
unit 98.14% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.21% <100.00%> (+<0.01%) ⬆️
OutsideTasks 97.92% <100.00%> (+<0.01%) ⬆️
Files Coverage Δ
database/models/core.py 97.28% <100.00%> (+0.18%) ⬆️
database/tests/unit/test_models.py 100.00% <100.00%> (ø)

Copy link

codecov-public-qa bot commented Jan 19, 2024

Codecov Report

Merging #236 (32a13ed) into main (c248c6f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #236   +/-   ##
=======================================
  Coverage   98.13%   98.14%           
=======================================
  Files         375      375           
  Lines       30667    30721   +54     
=======================================
+ Hits        30096    30150   +54     
  Misses        571      571           
Flag Coverage Δ
integration 98.14% <100.00%> (+<0.01%) ⬆️
latest-uploader-overall 98.14% <100.00%> (+<0.01%) ⬆️
unit 98.14% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.21% <100.00%> (+<0.01%) ⬆️
OutsideTasks 97.92% <100.00%> (+<0.01%) ⬆️
Files Coverage Δ
database/models/core.py 97.28% <100.00%> (+0.18%) ⬆️
database/tests/unit/test_models.py 100.00% <100.00%> (ø)

Copy link

codecov bot commented Jan 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c248c6f) 98.11% compared to head (32a13ed) 98.11%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #236   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files         406      406           
  Lines       31368    31422   +54     
=======================================
+ Hits        30776    30830   +54     
  Misses        592      592           
Flag Coverage Δ
integration 98.14% <100.00%> (+<0.01%) ⬆️
latest-uploader-overall 98.14% <100.00%> (+<0.01%) ⬆️
unit 98.14% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.12% <100.00%> (+<0.01%) ⬆️
OutsideTasks 97.92% <100.00%> (+<0.01%) ⬆️
Files Coverage Δ
database/models/core.py 97.28% <100.00%> (+0.18%) ⬆️
database/tests/unit/test_models.py 100.00% <100.00%> (ø)

This change has been scanned for critical changes. Learn more

giovanni-guidini added a commit that referenced this pull request Jan 23, 2024
depends on: #236

context: codecov/engineering-team#970

usage of the `GithubAppInstallation` model through the worker.
It still respects and doesn't affect deprecated usages of
`owner.integration_id` and `repo.using_integration`, but if a
ghapp exists for the owner it takes precedence over the legacy fields.
giovanni-guidini added a commit that referenced this pull request Jan 24, 2024
depends on: #236

context: codecov/engineering-team#970

usage of the `GithubAppInstallation` model through the worker.
It still respects and doesn't affect deprecated usages of
`owner.integration_id` and `repo.using_integration`, but if a
ghapp exists for the owner it takes precedence over the legacy fields.
These changes port codecov/codecov-api#346
to the worker so we can use them here as well.

context: codecov/engineering-team#970
giovanni-guidini added a commit that referenced this pull request Jan 26, 2024
depends on: #236

context: codecov/engineering-team#970

usage of the `GithubAppInstallation` model through the worker.
It still respects and doesn't affect deprecated usages of
`owner.integration_id` and `repo.using_integration`, but if a
ghapp exists for the owner it takes precedence over the legacy fields.
GITHUB_APP_INSTALLATION_DEFAULT_NAME = "codecov_app_installation"


class GithubAppInstallation(CodecovBaseModel, MixinBaseClass):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an easy way to test this E2E to prevent something like what happened in API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure. I don't think we have the infra in place...

I tested locally, if that's worth anything (upload, PR comment, statuses and exploring files in the UI), but it's very much a "trust me" scenario and we know it's easy to make mistakes.

@giovanni-guidini giovanni-guidini merged commit c8e780d into main Jan 29, 2024
31 checks passed
@giovanni-guidini giovanni-guidini deleted the gio/multi-apps/model branch January 29, 2024 12:39
giovanni-guidini added a commit that referenced this pull request Feb 1, 2024
depends on: #236

context: codecov/engineering-team#970

usage of the `GithubAppInstallation` model through the worker.
It still respects and doesn't affect deprecated usages of
`owner.integration_id` and `repo.using_integration`, but if a
ghapp exists for the owner it takes precedence over the legacy fields.
giovanni-guidini added a commit that referenced this pull request Feb 1, 2024
depends on: #236

context: codecov/engineering-team#970

usage of the `GithubAppInstallation` model through the worker.
It still respects and doesn't affect deprecated usages of
`owner.integration_id` and `repo.using_integration`, but if a
ghapp exists for the owner it takes precedence over the legacy fields.
This pull request was closed.
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