Skip to content

RK-19777 - remove sentry #77

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

Merged
merged 5 commits into from
Sep 27, 2023
Merged

RK-19777 - remove sentry #77

merged 5 commits into from
Sep 27, 2023

Conversation

ElDuderinos
Copy link
Contributor

No description provided.

@ElDuderinos
Copy link
Contributor Author

ElDuderinos commented Sep 10, 2023

Github Enforcer opened Task: RK-19777

@sonariorobot sonariorobot changed the title remove sentry RK-19777 - remove sentry Sep 10, 2023
from jaeger_client import Config
from flask_opentracing import FlaskTracer
from utils.logging import on_add_todo_logging, on_get_todos_logging
import os
from flask import send_from_directory

sentry_sdk.init(
Copy link
Contributor

Choose a reason for hiding this comment

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

Dont we need to recalculate the hash of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true true, fixed

@@ -6,19 +6,12 @@
from datetime import datetime
from random import randint
from todos_store import Store
import sentry_sdk
from sentry_sdk.integrations.flask import FlaskIntegration
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh no,
Just realized our e2es rely on things being in specific lines. So, removing lines will break e2es. Lets leave the removed lines empty insteqd of removing them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we do e2e with tutorial-python?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Urook ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ElDuderinos Yes, in profiling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, done

@ElDuderinos ElDuderinos requested a review from Urook September 27, 2023 08:21
app.py Outdated
@@ -7,17 +7,17 @@
from random import randint
from todos_store import Store
import sentry_sdk
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aha, sorry good catch, removed

@ElDuderinos ElDuderinos requested a review from Urook September 27, 2023 08:59
@sonariorobot sonariorobot merged commit 730cfbc into master Sep 27, 2023
@sonariorobot sonariorobot deleted the remove-sentry branch September 27, 2023 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants