ref(typing): type bitbucket issues file

[JoshFerge]

No description provided.

[asottile-sentry]

really weird that group is optional here?

[JoshFerge]

really weird that group is optional here?

I thought so too, i see where the function is called with None in a test.

sentry/tests/sentry/integrations/github/test_issues.py

Line 81 in 75e2057

config = install.get_create_issue_config(None, self.user, params={})

[asottile-sentry]

really weird that group is optional here?

I thought so too, i see where the function is called with None in a test.

sentry/tests/sentry/integrations/github/test_issues.py

Line 81 in 75e2057

config = install.get_create_issue_config(None, self.user, params={})

maybe the test should get fixed and then this can be non-nullable?

[JoshFerge]

sentry/tests/sentry/integrations/github/test_issues.py

Line 81 in 75e2057

config = install.get_create_issue_config(None, self.user, params={})

@leeandher do you know why we allow group to be None? it seems like this was intentional, but i don't see it being None anywhere in the code?

[leeandher]

sentry/tests/sentry/integrations/github/test_issues.py

Line 81 in 75e2057

config = install.get_create_issue_config(None, self.user, params={})

@leeandher do you know why we allow group to be None? it seems like this was intentional, but i don't see it being None anywhere in the code?

@JoshFerge To be honest, it's probably treated in some code as non-optional, and I can't really find anywhere that would make sense for it to None. When it's surfaced in the app, it is used to create issue links so we definitely have the context of a group.

Maybe theres something about removing the link from Sentry but still preserving something on the provider's side, but I'm kinda making that up.

I don't recall anything about typing it the first time, maybe the ecosystem team knows more

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #83644      +/-   ##
==========================================
- Coverage   87.63%   87.60%   -0.03%     
==========================================
  Files        9498     9495       -3     
  Lines      541781   539242    -2539     
  Branches    21241    21170      -71     
==========================================
- Hits       474771   472410    -2361     
+ Misses      66662    66487     -175     
+ Partials      348      345       -3     

[JoshFerge]

@leeandher @asottile-sentry after looking at other integrations, it appears they can expect group to be None,

sentry/src/sentry/integrations/jira/integration.py

Line 697 in 4170cbd

if group:

just seems like with bitbucket's integration it does not. I think this is okay to merge as is.

[asottile-sentry]

@leeandher @asottile-sentry after looking at other integrations, it appears they can expect group to be None,

sentry/src/sentry/integrations/jira/integration.py

Line 697 in 4170cbd

if group:

just seems like with bitbucket's integration it does not. I think this is okay to merge as is.

given IntegrationConfigSerializer I think this is going to hit the AssertionError -- something probably needs to be fixed there and we're getting lucky that nobody uses this thing with that api endpoint

[getsentry-bot]

PR reverted: 614cddc

[JoshFerge]

@leeandher @asottile-sentry after looking at other integrations, it appears they can expect group to be None,

sentry/src/sentry/integrations/jira/integration.py

Line 697 in 4170cbd

if group:

just seems like with bitbucket's integration it does not. I think this is okay to merge as is.

given IntegrationConfigSerializer I think this is going to hit the AssertionError -- something probably needs to be fixed there and we're getting lucky that nobody uses this thing with that api endpoint

gone ahead and reverted the PR. i'll put in a fix when i get a sec