dfreelon / pyktok

A simple module to collect video, text, and metadata from Tiktok.
BSD 3-Clause "New" or "Revised" License
316 stars 44 forks source link

Specifying a browser is not strictly needed #45

Closed tomasruizt closed 3 months ago

tomasruizt commented 3 months ago

Hi, thank you for sharing your code with the rest of us! I'm eager to build on top of this lib.

I noticed that I did not need to specify a browser to save tiktok videos. By defining the cookies=dict() within the pyktok code I was also able to fetch videos. Perhaps one could also pass pyktok.specify_browser(None), to set cookies=dict(). I'm happy to create a PR + test to enable this option, if it makes sense to you.

Background

I am running the function pyktok.save_tiktok() inside a docker container, and noticed that specyfing the firefox browser and installing it in the image was pretty painful. I needed to:

dfreelon commented 3 months ago

I haven't tested this, but assuming it works, it may be a new change--I originally added the browser req because TT wouldn't let me download anything without it. But sure, a PR would be great--I'll test it and add if it works for me. Thx

tomasruizt commented 3 months ago

I created a test case that passes with browser=None. If you can add me as a collaborator, I can create the pull request for you to review :) @dfreelon

dfreelon commented 3 months ago

Well I "assigned" you, and others have added PRs without any special privileges, so give it a try and LMK how it goes.

tomasruizt commented 3 months ago

I can create a pull request with only read access, but I cannot push my branch without write access (https://docs.github.com/en/desktop/working-with-your-remote-repository-on-github-or-github-enterprise/creating-an-issue-or-pull-request-from-github-desktop)

This is what I get trying to push my branch: image

Am I perhaps missing something?

dfreelon commented 3 months ago

Yeah, I have sole write access on this repo and that's by design. Just do your PR, I'll check it out and merge it if it works. Thx

tomasruizt commented 3 months ago

I'm not able to create a pull request. Here is a video of it: https://www.loom.com/share/911fbac9a12b466997402606f95e89e4?sid=ae022e06-3613-4375-b168-17fffa1dcb0a

dfreelon commented 3 months ago

Yeah you need to create your own branch and then request to merge with main. Here's an example of one I need to get to but haven't yet--still need to consider how best to integrate it: https://github.com/dfreelon/pyktok/pull/43

tomasruizt commented 3 months ago

I see! JIV-DLS created a pr from his own fork of the code. I'll try it out.

tomasruizt commented 3 months ago

Ok! Its looking good. I was able to create the PR and run its test case automatically on github actions.

dfreelon commented 3 months ago

Hi, thanks for the PR but I figured out there's an even easier way to implement this--just define the cookies global var as an empty dict at the start of the file and you don't have to run specify_browser at all (but you can if you want to). So I'll implement that today and close this issue.

dfreelon commented 3 months ago

OK, just pushed the changes to GitHub and PyPI. Feel free to check it out and LMK if it works for you. In the meantime, I will close this.

tomasruizt commented 3 months ago

perfect @dfreelon! That's even better :)