Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Conversation

@suejung-sentry
Copy link
Contributor

@suejung-sentry suejung-sentry commented Jan 13, 2025

In self-hosted / enterprise, our customers may not always have shelter deployed. Add a check for those signals that call shelter to confirm first that shelter is enabled.

This PR is paired with a similar fix done in api here to fix regenerateOrgUploadToken when shelter disabled.

Closes codecov/engineering-team#3212

@codecov-notifications
Copy link

codecov-notifications bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
database/events.py 77.77% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@codecov
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

Project coverage is 97.78%. Comparing base (ac302e7) to head (3fdebda).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
database/events.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1005      +/-   ##
==========================================
- Coverage   97.79%   97.78%   -0.01%     
==========================================
  Files         447      447              
  Lines       36175    36192      +17     
==========================================
+ Hits        35376    35391      +15     
- Misses        799      801       +2     
Flag Coverage Δ
integration 42.14% <35.29%> (-0.01%) ⬇️
unit 90.47% <88.23%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

⚠️ Impact Analysis from Codecov is deprecated and will be sunset on Jan 31 2025. See more

@github-actions
Copy link

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@suejung-sentry suejung-sentry force-pushed the sshin/3212 branch 2 times, most recently from c394b14 to a51a105 Compare January 13, 2025 21:33
@codecov-qa
Copy link

codecov-qa bot commented Jan 13, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
1772 1 1771 4
View the top 1 failed tests by shortest run time
database/tests/unit/test_events.py::test_shelter_repo_sync
Stack Traces | 0.048s run time
dbsession = <sqlalchemy.orm.session.Session object at 0x7fa5db116300>
mock_configuration = <shared.config.ConfigHelper object at 0x7fa5db115b80>
mocker = <pytest_mock.plugin.MockFixture object at 0x7fa5e03a1c10>

    def test_shelter_repo_sync(dbsession, mock_configuration, mocker):
        # this prevents the pubsub SDK from trying to load credentials
        os.environ["PUBSUB_EMULATOR_HOST"] = "localhost"
    
        mock_configuration.set_params(
            {
                "setup": {
                    "shelter": {
                        "pubsub_project_id": "test-project-id",
                        "sync_repo_topic_id": "test-topic-id",
                    }
                }
            }
        )
    
        publish = mocker.patch("google.cloud.pubsub_v1.PublisherClient.publish")
    
        # this triggers the publish via SQLAlchemy events (after_insert)
        repo = RepositoryFactory(repoid=91728376, name="test-123")
        dbsession.add(repo)
        dbsession.commit()
    
>       publish.assert_called_once_with(
            ".../test-project-id/topics/test-topic-id",
            b'{"type": "repo", "sync": "one", "id": 91728376}',
        )
E       AssertionError: Expected 'publish' to be called once. Called 0 times.

.../tests/unit/test_events.py:29: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@codecov-public-qa
Copy link

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
1772 1 1771 4
View the top 1 failed tests by shortest run time
database/tests/unit/test_events.py::::test_shelter_repo_sync
Stack Traces | 0.048s run time
dbsession = <sqlalchemy.orm.session.Session object at 0x7fa5db116300>
mock_configuration = <shared.config.ConfigHelper object at 0x7fa5db115b80>
mocker = <pytest_mock.plugin.MockFixture object at 0x7fa5e03a1c10>

    def test_shelter_repo_sync(dbsession, mock_configuration, mocker):
        # this prevents the pubsub SDK from trying to load credentials
        os.environ["PUBSUB_EMULATOR_HOST"] = "localhost"
    
        mock_configuration.set_params(
            {
                "setup": {
                    "shelter": {
                        "pubsub_project_id": "test-project-id",
                        "sync_repo_topic_id": "test-topic-id",
                    }
                }
            }
        )
    
        publish = mocker.patch("google.cloud.pubsub_v1.PublisherClient.publish")
    
        # this triggers the publish via SQLAlchemy events (after_insert)
        repo = RepositoryFactory(repoid=91728376, name="test-123")
        dbsession.add(repo)
        dbsession.commit()
    
>       publish.assert_called_once_with(
            ".../test-project-id/topics/test-topic-id",
            b'{"type": "repo", "sync": "one", "id": 91728376}',
        )
E       AssertionError: Expected 'publish' to be called once. Called 0 times.

.../tests/unit/test_events.py:29: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@suejung-sentry suejung-sentry added this pull request to the merge queue Jan 15, 2025
Merged via the queue into main with commit 6080307 Jan 15, 2025
16 of 27 checks passed
@suejung-sentry suejung-sentry deleted the sshin/3212 branch January 15, 2025 19:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Worker] Only emit shelter pubsub when shelter is enabled

3 participants