-
Notifications
You must be signed in to change notification settings - Fork 28
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
ref: Accept service less requests gq #187
Conversation
b99affe
to
6cfa530
Compare
6cfa530
to
17521c1
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #187 +/- ##
=======================================
Coverage 95.62% 95.62%
=======================================
Files 600 600
Lines 15209 15219 +10
=======================================
+ Hits 14543 14553 +10
Misses 666 666
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report
@@ Coverage Diff @@
## main #187 +/- ##
=======================================
Coverage 95.62% 95.62%
=======================================
Files 600 600
Lines 15209 15219 +10
=======================================
+ Hits 14543 14553 +10
Misses 666 666
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov Report
@@ Coverage Diff @@
## main #187 +/- ##
=====================================
Coverage 95.52 95.52
=====================================
Files 714 714
Lines 15615 15625 +10
=====================================
+ Hits 14915 14925 +10
Misses 700 700
Flags with carried forward coverage won't be shown. Click here to find out more.
|
graphql_api/tests/test_owner.py
Outdated
self.gql_request(query, provider="", owner=current_org) | ||
except MissingService as e: | ||
assert str(e) == "Missing service" | ||
raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this test is passing then maybe this error is not being rased as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the test would error if this exception was indeed being raised
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait i see what you mean, looking into it
Purpose/Motivation
What is the feature? Why is this being done?
To be able to use TOS with service less requests, we need to update the mutation (graphql endpoints) to accept no service, and raise a proper exception otherwise.
What does this PR do?
Include a brief description of the changes in this PR. Bullet points are your friend.
Notes to Reviewer
Anything to note to the team? Any tips on how to review, or where to start?
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. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this 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.