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

Add scripts for automated social posting #477

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

rleed
Copy link
Contributor

@rleed rleed commented Sep 6, 2023

Should close #434. README.md says a lot. This basically just curls the GraphQL api and generates tweets/events from that. Ported to js, see conv below.

@ekzyis ekzyis added the feature new product features that weren't there before label Sep 7, 2023
@SatsAllDay
Copy link
Contributor

Not sure how to create an ad yet for testing. (Hints, anyone?)

Can you just insert items into the database manually, and associate them to the ad account?

@ekzyis
Copy link
Member

ekzyis commented Sep 7, 2023

I haven't looked through all of the code but my first impression is that I don't think we want to maintain a lot of bash code.

Don't get me wrong, I like bash, but not 299 lines of it, lol.

Imo, it's a good scripting language for "quick and dirty" solutions for one-time jobs or for one-liners using a clever combination of CLI tools thanks to the UNIX philosophy (do one thing and do it well) where other scripting languages like Python or NodeJS may be more verbose. But I think it's not the right choice for this task here:

  • It's hard to read: for example, -z stands for "check if empty" - I even had to look this up to make sure even though I would consider myself to have quite some experience with bash
  • It's error prone: forget some " and you may run into a hard to find bug since it only happens if there is whitespace in a variable or even worse: arbitrary code execution
  • I think not many devs have a lot of experience with bash
  • probably other reasons that I don't remember right now

Also, why split these into different files? If you really want to go down this path (without guarantees that we will merge this), you should define functions in these different files which you can source in a single bash file. Then you don't need to call one script from the other script.

Also, I am not sure if we can install command line tools in the environment in which SN is deployed.

I think all of this code should be placed in the worker.

@rleed
Copy link
Contributor Author

rleed commented Sep 7, 2023

Not sure how to create an ad yet for testing. (Hints, anyone?)

Can you just insert items into the database manually, and associate them to the ad account?

Yeah, I searched the code and figured out how to do it...just set the "Item"."userId" to 9 (the global ad user). Now the script also checks that to filter out ads. So the functionality is now complete.

@rleed
Copy link
Contributor Author

rleed commented Sep 7, 2023

I think all of this code should be placed in the worker.

Lol that was my original plan, but I thought this was supposed to be independent of the SN site. Perhaps being in the worker is independent enough...from the app portion at least? @huumn

@rleed
Copy link
Contributor Author

rleed commented Sep 8, 2023

If we do this in the worker, what do you think about adding twitter-api-v2 and blastr as the dependencies?

@rleed
Copy link
Contributor Author

rleed commented Sep 9, 2023

If we do this in the worker, what do you think about adding twitter-api-v2 and blastr as the dependencies?

I guess for completeness it's worth mentioning some of the benefits of the bash solution:

  • The twurl and noscl command-line tools are likely more stable as dependencies, since they are designed for bots, exactly for this type of use case, and they are end-user tools that seem to be widely used and relied upon.
  • The main maintenance surface would be around the GraphQL API. (I don't know how much the fields it depends on might change in the future, but maybe you have a feel for that. I wouldn't necessarily expect those fields to be changing much.)
  • The bash solution is completely stand-alone and isn't even required to run on the same server.
  • Minor maintenance (such as changing the scheduling or wording of the tweets) would arguably be easier in bash.

Just FWIW....not intending to dig heels in if the sentiment is strong the other way.

@huumn
Copy link
Member

huumn commented Sep 11, 2023

While this technically satisfies the issue, I'd strongly prefer these were written in JS like everything else.

If we do this in the worker, what do you think about adding twitter-api-v2 and blastr as the dependencies?

It need not be in the worker, but if it can be if that's easier. Ideally, we'd run this as its own microservice.

As for deps, we'd likely only need a twitter api one and some websocket lib that allows us to send events to wss://nostr.mutinywallet.com.

I'm going to move this to a draft pending conversion to javascript. @rleed if you're not interested in completing the conversion I can partially compensate you for the work here. You've logically modeled the solution.

@huumn huumn marked this pull request as draft September 11, 2023 20:48
@rleed
Copy link
Contributor Author

rleed commented Sep 11, 2023

I'm going to move this to a draft pending conversion to javascript. @rleed if you're not interested in completing the conversion I can partially compensate you for the work here. You've logically modeled the solution.

Interested!? More than happy to finish the job! (Honestly it would be nice to have partial compensation now, but I'd also be delighted to take it across the finish line too.)

Thanks for weighing in...

@huumn
Copy link
Member

huumn commented Sep 11, 2023

Yeah sorry for the late reply. Been conferencing for the last week.

I'm reviewing your other straightforward PRs now and I'll send along funds for them all at once.

@rleed
Copy link
Contributor Author

rleed commented Sep 12, 2023

Ideally, we'd run this as its own microservice.

I've added a new container for it and ported the main logic already.

As for deps, we'd likely only need a twitter api one and some websocket lib that allows us to send events to wss://nostr.mutinywallet.com.

Sounds good. I'm working on that now...

@rleed
Copy link
Contributor Author

rleed commented Sep 12, 2023

This is now fully functional!

That was fun...

  • I didn't notice till now that we already have a nostr dependency, which works beautifully.
  • The twitter dependency I added has zero additional dependencies, and it works great too, so I think it was a good choice.
  • The PgBoss scheduling simplified things a lot.
  • Everything is nicer.

In the end, I'm soo happy we did this in JS. Simply nicer. @ekzyis, you were right from the beginning ;)

@huumn
Copy link
Member

huumn commented Sep 12, 2023

Looks great at a glance!

I'm going to deploy some other stuff, but I'll send you the partial sum as discussed along with the other stuff once I've deployed the stuff I've already reviewed.

@huumn huumn self-assigned this Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new product features that weren't there before
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automated Social Posting (500,000 Sat Bounty)
4 participants