Skip to content

Enterprise ID does not always show up so checked for it#352

Closed
MicahLyle wants to merge 2 commits into
slackapi:masterfrom
MicahLyle:master
Closed

Enterprise ID does not always show up so checked for it#352
MicahLyle wants to merge 2 commits into
slackapi:masterfrom
MicahLyle:master

Conversation

@MicahLyle

@MicahLyle MicahLyle commented Sep 18, 2018

Copy link
Copy Markdown

Summary

Describe the goal of this PR. Mention any related Issue numbers.

Handles cases when enterprise_id is not available, like below:

    response = client.api_call(method="users.info", user=user_id)
  File "C:\Users\MICAHL~1\Envs\ch\lib\site-packages\slackclient\client.py", line 181, in api_call
    self.refresh_access_token()
  File "C:\Users\MICAHL~1\Envs\ch\lib\site-packages\slackclient\client.py", line 109, in refresh_access_token
    "enterprise_id": response_json["enterprise_id"],
KeyError: 'enterprise_id'

Requirements (place an x in each [ ])

@codecov

codecov Bot commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #352 into master will decrease coverage by 0.12%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
- Coverage   76.38%   76.25%   -0.13%     
==========================================
  Files          10       10              
  Lines         415      417       +2     
  Branches       74       75       +1     
==========================================
+ Hits          317      318       +1     
  Misses         87       87              
- Partials       11       12       +1
Impacted Files Coverage Δ
slackclient/client.py 66.32% <50%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 669cb54...ad37106. Read the comment docs.

@codecov

codecov Bot commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #352 into master will decrease coverage by 0.43%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
- Coverage   76.38%   75.95%   -0.44%     
==========================================
  Files          10       10              
  Lines         415      420       +5     
  Branches       74       76       +2     
==========================================
+ Hits          317      319       +2     
- Misses         87       88       +1     
- Partials       11       13       +2
Impacted Files Coverage Δ
slackclient/server.py 84.18% <100%> (+0.08%) ⬆️
slackclient/slackrequest.py 95.65% <50%> (-4.35%) ⬇️
slackclient/client.py 66.32% <50%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 669cb54...ad37106. Read the comment docs.

4 similar comments
@codecov

codecov Bot commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #352 into master will decrease coverage by 0.43%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
- Coverage   76.38%   75.95%   -0.44%     
==========================================
  Files          10       10              
  Lines         415      420       +5     
  Branches       74       76       +2     
==========================================
+ Hits          317      319       +2     
- Misses         87       88       +1     
- Partials       11       13       +2
Impacted Files Coverage Δ
slackclient/server.py 84.18% <100%> (+0.08%) ⬆️
slackclient/slackrequest.py 95.65% <50%> (-4.35%) ⬇️
slackclient/client.py 66.32% <50%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 669cb54...ad37106. Read the comment docs.

@codecov

codecov Bot commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #352 into master will decrease coverage by 0.43%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
- Coverage   76.38%   75.95%   -0.44%     
==========================================
  Files          10       10              
  Lines         415      420       +5     
  Branches       74       76       +2     
==========================================
+ Hits          317      319       +2     
- Misses         87       88       +1     
- Partials       11       13       +2
Impacted Files Coverage Δ
slackclient/server.py 84.18% <100%> (+0.08%) ⬆️
slackclient/slackrequest.py 95.65% <50%> (-4.35%) ⬇️
slackclient/client.py 66.32% <50%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 669cb54...ad37106. Read the comment docs.

@codecov

codecov Bot commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #352 into master will decrease coverage by 0.43%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
- Coverage   76.38%   75.95%   -0.44%     
==========================================
  Files          10       10              
  Lines         415      420       +5     
  Branches       74       76       +2     
==========================================
+ Hits          317      319       +2     
- Misses         87       88       +1     
- Partials       11       13       +2
Impacted Files Coverage Δ
slackclient/server.py 84.18% <100%> (+0.08%) ⬆️
slackclient/slackrequest.py 95.65% <50%> (-4.35%) ⬇️
slackclient/client.py 66.32% <50%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 669cb54...ad37106. Read the comment docs.

@codecov

codecov Bot commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #352 into master will decrease coverage by 0.43%.
The diff coverage is 62.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #352      +/-   ##
==========================================
- Coverage   76.38%   75.95%   -0.44%     
==========================================
  Files          10       10              
  Lines         415      420       +5     
  Branches       74       76       +2     
==========================================
+ Hits          317      319       +2     
- Misses         87       88       +1     
- Partials       11       13       +2
Impacted Files Coverage Δ
slackclient/server.py 84.18% <100%> (+0.08%) ⬆️
slackclient/slackrequest.py 95.65% <50%> (-4.35%) ⬇️
slackclient/client.py 66.32% <50%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 669cb54...ad37106. Read the comment docs.

@Roach

Roach commented Sep 27, 2018

Copy link
Copy Markdown
Contributor

@MicahLyle thanks for pointing this out! This is an issue with the API on Slack's side - we have an open ticket to fix it.

@aoberoi

aoberoi commented Sep 28, 2018

Copy link
Copy Markdown
Contributor

@Roach 🤔 whether or not the platform ships a fix, we should solve this at the SDK level so that it at least doesn’t raise an exception. Otherwise there’s no good way for anyone to use this lib at all until the platform fixes. Does that sound reasonable?

@Roach

Roach commented Sep 28, 2018

Copy link
Copy Markdown
Contributor

@aoberoi @MicahLyle it looks like this has been fixed in the API. Would you mind testing your script again to confirm that you're no longer getting that error?

@MicahLyle

MicahLyle commented Sep 28, 2018

Copy link
Copy Markdown
Author

@Roach I'm actually not going with workspace apps yet because they're not ready for my use case, so I switched some of the backend code to use the regular slack client instead of the workspace one, so right now I don't know if it works or not, haven't used the workspace code in over a week and a half and I'd have to like reset the database and stuff to test it.

If you and/or anyone else would be interested in hearing some limitations/use cases for workspace apps that I think were pretty unique and why I felt like I couldn't use them yet though I'd be happy to talk with anyone at Slack!

@seratch

seratch commented Apr 9, 2020

Copy link
Copy Markdown
Contributor

Thank you very much for taking the time to make this PR. This project no longer supports version 1.x as we've described here. Allow me to close this now. If you see the necessity to fix something similar in v2, please let us know by raising a new issue.

@seratch seratch closed this Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MicahLyle @Roach @aoberoi @seratch