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

Follow pagination for >500 emoji #23

Merged
merged 2 commits into from
Jun 21, 2018
Merged

Follow pagination for >500 emoji #23

merged 2 commits into from
Jun 21, 2018

Conversation

d-lord
Copy link
Collaborator

@d-lord d-lord commented Jun 12, 2018

Hi! A friend mentioned that the export script wasn't catching every emoji any more.

At some point, Slack started paginating on the export page for teams with huge emoji collections.
This means the export process would only detect the 500 emoji on that first page (ordered alphabetically).
It now looks to other pages and exports those as well, restoring full functionality.

Tested on a team with ~1300 emoji (3 pages).

(I've also commented/renamed/added type signatures to methods & ran PyCharm's built-in "optimize imports" to tidy that, hence the changes at the top of the file)

At some point, Slack started paginating on the export page for teams with huge emoji collections.
This means the export process would only detect the 500 emoji on that first page (ordered alphabetically).
It now looks to other pages and exports those as well, restoring full functionality.

Tested on a team with ~1300 emoji (3 pages).
@mitchmcdee
Copy link

I'm successfully parsing all 1300 emojis now :) however, I still get this error:

Traceback (most recent call last):
  File "export.py", line 128, in <module>
    loop.run_until_complete(main())
  File "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/asyncio/base_events.py", line 468, in run_until_complete
    return future.result()
  File "export.py", line 118, in main
    http_get = concurrent_http_get(args.concurrent_requests, session)
  File "export.py", line 49, in concurrent_http_get
    semaphore = asyncio.Semaphore(max_concurrent)
  File "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/asyncio/locks.py", line 426, in __init__
    if value < 0:
TypeError: '<' not supported between instances of 'str' and 'int'

@mitchmcdee
Copy link

fyi I fixed this error by wrapping max_concurrent with int() lol, would have thought typing would have caught that?

@mitchmcdee
Copy link

Why isn't upload async? :o

@mitchmcdee
Copy link

apart from that looks like everything worked! :) thank you!

It comes through from the environment variable as a string. Fixes this
unhelpful error message:

`TypeError: '<' not supported between instances of 'str' and 'int'`
@d-lord
Copy link
Collaborator Author

d-lord commented Jun 12, 2018

I guess you're using the CONCURRENT_REQUESTS environment variable? Looks like I didn't test that at all 😩 It comes in as a string. I've reproduced & fixed that, please try again.

@mitchmcdee
Copy link

hang on I'm doing bulk upload bahahah, but that was my issue so I'm guessing its fixed! still confused why typing didn't crap its pants? was it meant to?

@d-lord
Copy link
Collaborator Author

d-lord commented Jun 12, 2018

My understanding is that typing is for static analysis, and is pretty toothless at runtime. Guessing the static analysis doesn't pick up on "argparse namespaced attribute may not be an int".

As to why upload isn't async - I haven't bothered! When I added async to export, my home internet could do heaps more than one simultaneous download, but had horrendous upload. This is @smashwilson's project and he already had a perfectly good upload script for my needs 😇

@d-lord
Copy link
Collaborator Author

d-lord commented Jun 20, 2018

Hi @smashwilson, any objection to me merging this on the main branch?

@smashwilson
Copy link
Owner

@d-lord None at all, merge at will 🚀

@d-lord d-lord merged commit 4f98dc1 into master Jun 21, 2018
@d-lord d-lord deleted the pagination-fix branch June 21, 2018 03:43
@d-lord
Copy link
Collaborator Author

d-lord commented Jun 21, 2018

Cheers! ⚡️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants