Add logic to process actors and posts #43

lazyguru · 2024-02-22T06:50:22Z

This PR adds a lot of things. For one, it introduces handling of users, communities, and posts at a basic level from the backend via queue (RabbitMQ). It also adds some basic activitypub support (in terms of request URL handling, not of pushing messages out). It, unfortunately, has grown quite large and really needs to get reviewed+merged so other work can continue. If necessary, I an attempt to break it up into smaller PRs. There are some other minor little commits that I've snuck in as well (namely, adding air for hot-reload support, json-schema stuff, switching to Gorm)

github-actions · 2024-02-22T06:51:05Z

Go test coverage: 0.3% for commit 1c2da88

⚠️ 9 of 10 packages have zero coverage.

sublinks/sublinks-federation
sublinks/sublinks-federation/cmd
sublinks/sublinks-federation/internal/activitypub
sublinks/sublinks-federation/internal/db
sublinks/sublinks-federation/internal/http
sublinks/sublinks-federation/internal/log
sublinks/sublinks-federation/internal/queue
sublinks/sublinks-federation/internal/repository
sublinks/sublinks-federation/internal/worker

View coverage for all packages

# Package Name                                      | Coverage
- sublinks/sublinks-federation                      |     0.0%
- sublinks/sublinks-federation/cmd                  |     0.0%
- sublinks/sublinks-federation/internal/activitypub |     0.0%
- sublinks/sublinks-federation/internal/db          |     0.0%
- sublinks/sublinks-federation/internal/http        |     0.0%
+ sublinks/sublinks-federation/internal/lemmy       |     2.7%
- sublinks/sublinks-federation/internal/log         |     0.0%
- sublinks/sublinks-federation/internal/queue       |     0.0%
- sublinks/sublinks-federation/internal/repository  |     0.0%
- sublinks/sublinks-federation/internal/worker      |     0.0%

lazyguru · 2024-02-22T06:51:09Z

@devanbenz would appreciate some feedback here. Not sure I am done though as I may take another pass at it

lazyguru · 2024-03-04T05:01:57Z

I have other code locally, but I think I can push that all to a separate PR and pause additional work here so we can get this one approved+merged first

lazyguru · 2024-03-04T05:02:53Z

cmd/federation.go

-		logger.Fatal("failed starting to consumer", err)
-	}
+	processActors(logger, conn, q)
+	processPosts(logger, conn, q)


These probably need to be moved somewhere else eventually

We could probably encapsulate a lot of the queue logic into the actual queue.go file and have like a single function called Run() or RunRabbitMq() that can do all the work.

Yeah, that might make sense. I've been trying to keep things separate where possible (eg not have business logic in the queue code but rather have that somewhere else), but I also don't want to over-engineer this either.

😦 That doesn't work. I took a stab at refactoring to have it run inside the queue. The problem is I need the database there also. So now I have to pass the database to the queue and that feels icky. I think I need to create another object and pass that object the db, logger, and queue (like we do the http server).

I managed to rework things to move these commands into the queue namespace. Not sure that makes sense as a final resting place, but probably good for now

lazyguru · 2024-03-04T05:03:37Z

cmd/federation.go

+	}
+}
+
+func processPosts(logger *log.Log, conn db.Database, q queue.Queue) {


This can maybe be made more generic using some of the code from #46

Which reminds me... @devanbenz don't delete your branch just yet 😄

Sounds good :D

internal/activitypub/person.go

internal/db/migrate.go

lazyguru · 2024-03-04T05:13:23Z

internal/queue/consumer.go

-		false,     // no wait
-		nil,       // arguments
+		queueData.QueueName, // queue name
+		"",                  // consumer


When this is blank, it gets auto-assigned. Might be ok? But might also be a better idea for us to think this through some more

Could set the consumer to a unique id with the routing key included in the string?

Maybe. Like I said. Need to think it through. Probably, should create a separate issue to track it after this PR gets merged

lazyguru · 2024-03-04T05:15:43Z

internal/queue/consumer.go

-		nil,       // arguments
+		queueData.QueueName, // queue name
+		"",                  // consumer
+		false,               // auto-ack


The manual ack logic I have right now has issues. If you Nack a message, it just shoves it back in the queue and you reattempt to process it next time it's turn comes up. Need to investigate this more as it just creates an endless loop in local dev. I suspect there is a counter on the message somewhere that you can use to see how many times you've seen it. Also, a "retry after X period of time" would be nice and be able to apply an exponential backoff strategy

We should really implement a dead letter exchange. Let me know what you think? It would be beneficial for issues like this.

lazyguru · 2024-03-04T05:16:44Z

internal/queue/consumer.go

+			if err != nil {
+				err = message.Acknowledger.Nack(message.DeliveryTag, false, true)
+				if err != nil {
+					panic(err) // I know this isn't good. Will need to fix it


It's really not good considering the docker image is set to "restart: always" meaning we still get stuck in an endless loop

Lets think about a way to create a worker pool for this and that way workers can die without causing the entire application to crash. I have a very basic worker pool library that I created a little while ago: https://pkg.go.dev/github.com/we-are-discussing-rest/pool-party I can modify it to have logic to kill off workers

Or at least we can still implement a dead letter exchange but add something like an array of servers that it failed to send to so that way we can retry again for the instances where it failed only

Yeah but this is internal communication. Any failures would be due to bugs in ours or the API server code

lazyguru · 2024-03-04T05:17:34Z

internal/queue/consumer.go

+			}
+			err = message.Acknowledger.Ack(message.DeliveryTag, false)
+			if err != nil {
+				panic(err) // I know this isn't good. Will need to fix it


This is maybe not as bad considering we really shouldn't run into errors trying to ack a message, but...

lazyguru · 2024-03-04T05:19:32Z

internal/repository/repository.go

+	Find(*interface{}, ...interface{}) error
+}
+
+type RepositoryImpl struct {


I ended up going with a generic implementation as the only thing that was different in each repository was the the type reference. This just led to a bunch of boilerplate copy-paste code that made my skin crawl 😱

devanbenz

Left some comments

devanbenz · 2024-03-04T15:05:32Z

internal/db/db.go

 }

 type PostgresDB struct {
-	*sql.DB
+	*gorm.DB


Are we going to be doing any SQL queries that are more advanced than a few SELECTS or INSERTS? I am not a big fan of incorporating gorm I think the base SQL driver has great functionality. Like to get your opinion on this though.

I would not expect to. An ORM should be the better way to go (I know Go devs don't typically like dependencies, but writing your own SQL is not a good idea unless you really understand how databases work and have AND use proper indexes)

I heard someone else complain about Gorm recently. Is there another ORM that we should use? I'm generally opposed to not using an ORM as I'm of the personal opinion that too often developers think they should use SQL because they are developers but really they shouldn't because using SQL is more than just syntax, you need to understand the specific database internals and structure of the tables to write proper SQL. An ORM "saves" you from this as ORMs are typically built to manage the structure of the database with the understanding of the specific database (MySQL / Postgres / etc...) you are working with. (also, ORM makes it easier to swap out to a different DB in the future as you avoid using syntax that is specific to one database)

I've heard very good things about Ent. That being said I'm pretty sure SQLx works with any relational datastore and allows for schema-first querying.

internal/db/migrate.go

devanbenz · 2024-03-04T15:12:20Z

cmd/federation.go

-		logger.Fatal("failed starting to consumer", err)
-	}
+	processActors(logger, conn, q)
+	processPosts(logger, conn, q)


We could probably encapsulate a lot of the queue logic into the actual queue.go file and have like a single function called Run() or RunRabbitMq() that can do all the work.

internal/activitypub/person.go

internal/http/server.go

devanbenz · 2024-03-04T15:23:28Z

internal/queue/consumer.go

+			if err != nil {
+				err = message.Acknowledger.Nack(message.DeliveryTag, false, true)
+				if err != nil {
+					panic(err) // I know this isn't good. Will need to fix it


Lets think about a way to create a worker pool for this and that way workers can die without causing the entire application to crash. I have a very basic worker pool library that I created a little while ago: https://pkg.go.dev/github.com/we-are-discussing-rest/pool-party I can modify it to have logic to kill off workers

devanbenz · 2024-03-04T15:26:51Z

internal/queue/consumer.go

-		false,     // no wait
-		nil,       // arguments
+		queueData.QueueName, // queue name
+		"",                  // consumer


Could set the consumer to a unique id with the routing key included in the string?

devanbenz · 2024-03-04T16:04:40Z

internal/queue/consumer.go

+			if err != nil {
+				err = message.Acknowledger.Nack(message.DeliveryTag, false, true)
+				if err != nil {
+					panic(err) // I know this isn't good. Will need to fix it


Or at least we can still implement a dead letter exchange but add something like an array of servers that it failed to send to so that way we can retry again for the instances where it failed only

devanbenz

code looks good we just need to think about whether to keep using gorm or another thing and how to handle some of the errors within the messages.

devanbenz · 2024-03-16T22:05:47Z

internal/queue/consumer.go

-		nil,       // arguments
+		queueData.QueueName, // queue name
+		"",                  // consumer
+		false,               // auto-ack


We should really implement a dead letter exchange. Let me know what you think? It would be beneficial for issues like this.

lazyguru self-assigned this Feb 22, 2024

lazyguru force-pushed the add-logic-to-process-actors branch from 34bc4c3 to a341667 Compare February 23, 2024 07:25

lazyguru mentioned this pull request Mar 4, 2024

feat: 45: Adjust consumer for multiple routing keys #46

Closed

lazyguru marked this pull request as ready for review March 4, 2024 05:00

lazyguru requested a review from devanbenz March 4, 2024 05:01

lazyguru commented Mar 4, 2024

View reviewed changes

internal/activitypub/person.go Outdated Show resolved Hide resolved

lazyguru commented Mar 4, 2024

View reviewed changes

internal/activitypub/person.go Outdated Show resolved Hide resolved

lazyguru commented Mar 4, 2024

View reviewed changes

internal/db/migrate.go Show resolved Hide resolved

lazyguru commented Mar 4, 2024

View reviewed changes

lazyguru changed the title ~~WIP: Add logic to process actors~~ Add logic to process actors Mar 4, 2024

lazyguru changed the title ~~Add logic to process actors~~ Add logic to process actors and posts Mar 4, 2024

lazyguru force-pushed the add-logic-to-process-actors branch from 506c17c to 1e1a47d Compare March 6, 2024 22:34

devanbenz requested changes Mar 6, 2024

View reviewed changes

lazyguru force-pushed the add-logic-to-process-actors branch 2 times, most recently from 632b8a3 to 5ae0fb7 Compare March 7, 2024 04:23

Add air config for hot reloading during dev

6594186

lazyguru force-pushed the add-logic-to-process-actors branch from 5ae0fb7 to a1d9848 Compare March 15, 2024 05:51

lazyguru added 4 commits March 15, 2024 00:48

Add environment variable to control host+port

348c18f

Change to using Gorm for DB

b6925dd

Add repository interface and generic repository implementation

4360d1f

Refactor queue consumer code a little

3f1e678

lazyguru added 5 commits March 15, 2024 00:48

Add initial version of JSON Schema files for actor and post

5876bfc

Adjust schema to add additional fields

1c6375e

Refactor code to make use of workers instead of embedding queue logic

9744db8

Fix inbox/outbox URLs

42a7727

Move all queue processing inside queue code

7206f75

lazyguru force-pushed the add-logic-to-process-actors branch from a1d9848 to 7206f75 Compare March 15, 2024 06:48

lazyguru requested review from devanbenz and a team March 15, 2024 06:57

devanbenz approved these changes Mar 16, 2024

View reviewed changes

lazyguru merged commit d7814e7 into main Mar 16, 2024
2 checks passed

lazyguru deleted the add-logic-to-process-actors branch March 16, 2024 22:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logic to process actors and posts #43

Add logic to process actors and posts #43

lazyguru commented Feb 22, 2024 •

edited

Loading

github-actions bot commented Feb 22, 2024 •

edited

Loading

lazyguru commented Feb 22, 2024

lazyguru commented Mar 4, 2024

lazyguru Mar 4, 2024

devanbenz Mar 4, 2024

lazyguru Mar 7, 2024

lazyguru Mar 7, 2024

lazyguru Mar 15, 2024

lazyguru Mar 4, 2024

lazyguru Mar 4, 2024

devanbenz Mar 4, 2024

lazyguru Mar 4, 2024

devanbenz Mar 4, 2024

lazyguru Mar 7, 2024

lazyguru Mar 4, 2024

devanbenz Mar 16, 2024

lazyguru Mar 4, 2024

devanbenz Mar 4, 2024

devanbenz Mar 4, 2024

lazyguru Mar 7, 2024

lazyguru Mar 4, 2024

lazyguru Mar 4, 2024

devanbenz left a comment

devanbenz Mar 4, 2024

lazyguru Mar 7, 2024

lazyguru Mar 15, 2024

devanbenz Mar 16, 2024

devanbenz Mar 4, 2024

devanbenz Mar 4, 2024

devanbenz Mar 4, 2024

devanbenz Mar 4, 2024

devanbenz left a comment

devanbenz Mar 16, 2024

Add logic to process actors and posts #43

Add logic to process actors and posts #43

Conversation

lazyguru commented Feb 22, 2024 • edited Loading

github-actions bot commented Feb 22, 2024 • edited Loading

lazyguru commented Feb 22, 2024

lazyguru commented Mar 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devanbenz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devanbenz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lazyguru commented Feb 22, 2024 •

edited

Loading

github-actions bot commented Feb 22, 2024 •

edited

Loading