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

fix(django_admin): Disable django_admin on prod #52329

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

klboke
Copy link
Contributor

@klboke klboke commented Jul 6, 2023

Disable django_admin on prod ,fix #23742 , getsentry/self-hosted#1039

cc @BYK

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 6, 2023
@klboke
Copy link
Contributor Author

klboke commented Jul 6, 2023

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #52329 (09a8f22) into master (d285698) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #52329      +/-   ##
==========================================
- Coverage   79.32%   79.31%   -0.01%     
==========================================
  Files        4907     4907              
  Lines      205595   205595              
  Branches    35148    35148              
==========================================
- Hits       163081   163074       -7     
- Misses      37534    37540       +6     
- Partials     4980     4981       +1     
Impacted Files Coverage Δ
src/sentry/conf/urls.py 78.57% <0.00%> (-14.29%) ⬇️
static/app/views/issueDetails/actions/index.tsx 50.00% <ø> (ø)
static/app/views/issueList/actions/actionSet.tsx 77.08% <ø> (ø)
...tatic/app/views/issueList/actions/reviewAction.tsx 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Lol, was that it? 😅

Can we have a smoke test somewhere so we know for sure it is fixed and will stay that way?

@hubertdeng123
Copy link
Member

/gcbrun

@hubertdeng123
Copy link
Member

Lol, was that it? 😅

Seems to work, just tested this locally. Thanks @klboke

Copy link
Member

@hubertdeng123 hubertdeng123 left a comment

Choose a reason for hiding this comment

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

Can we have a smoke test somewhere so we know for sure it is fixed and will stay that way?

I think this is a nice thing to have but don't think we should block on this.

@chadwhitacre chadwhitacre requested a review from a team July 7, 2023 21:01
@chadwhitacre
Copy link
Member

Let's get a review from the backend team as well.

@chadwhitacre chadwhitacre added the Trigger: getsentry tests once code is reviewed: apply label to PR to trigger getsentry tests label Jul 10, 2023
@hubertdeng123 hubertdeng123 merged commit 85a4fdb into getsentry:master Jul 10, 2023
62 of 64 checks passed
markstory added a commit that referenced this pull request Jul 11, 2023
We haven't used django-admin in a long time and it was recently disabled
in self-hosted (#52329)

Given that we don't use it in saas, and it is not part of any common
development workflows I think it is time to not include django-admin in
our installed app list.
@klboke klboke deleted the kl-fix branch July 11, 2023 23:23
markstory added a commit that referenced this pull request Jul 12, 2023
We haven't used django-admin in a long time and it was recently disabled
in self-hosted (#52329)

Given that we don't use it in saas, and it is not part of any common
development workflows I think it is time to not include django-admin in
our installed app list.

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: getsentry tests once code is reviewed: apply label to PR to trigger getsentry tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants