Moves the definition of the send function to resolve issues with kafka related imports in the remix build

[dakota002]

Temporary solution for #2588

It looks like the issue started with this commit

This PR moves the definition of the send function so it can still be reusable and should remove any dependency that would have caused the .node library to be needed.

The next step is to figure out the best way to include the zstd library in the build, as future updates will possibly require it for more Kafka functionality

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 28 lines in your changes missing coverage. Please review.

Project coverage is 6.09%. Comparing base (54ea8ce) to head (698ce96).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
app/routes/circulars/circulars.server.ts	0.00%	27 Missing ⚠️
app/table-streams/circulars/index.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #2590   +/-   ##
=====================================
  Coverage   6.08%   6.09%           
=====================================
  Files        164     164           
  Lines       4077    4072    -5     
  Branches     450     450           
=====================================
  Hits         248     248           
+ Misses      3827    3822    -5     
  Partials       2       2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lpsinger]

This won't fix the issue. The problem is that the zstd shared library is not getting bundled.

[dakota002]

This won't fix the issue. The problem is that the zstd shared library is not getting bundled.

I'm investigating this as an option for the first possible solution you mentioned.
"Ensure that it is always the table stream Lambda that is sending Kafka records"

The table-stream lambda already is the only thing sending Kafka records. But, since it was exporting a function that gets used in the remix lambda, I'm trying to determine whether or not that meant that its dependencies are getting tangled up in an unintended way.

The main issue that I am seeing though is that this may be suggesting the problem is also going to be present in #2301

Still investigating though, I think the better solution will be to figure out the inclusion of the lib, but in the meantime this may be a good option to unblock the broken deployments

[lpsinger]

I'm investigating this as an option for the first possible solution you mentioned.

I understand now. Go for it!

[lpsinger]

How will this fix the issue?

[dakota002]

How will this fix the issue?

I linked the commit that caused this in the description at the top. Specifically I believe that exporting/importing the send function from app/table-streams/circulars/index.ts caused the dependencies (the zstd node lib required by gcn-kafka) to be required by the main build.

Moving the send function out should resolve the issue, since this will be back, more or less, to a successful build state

[lpsinger]

How will this fix the issue?

I linked the commit that caused this in the description at the top. Specifically I believe that exporting/importing the send function from app/table-streams/circulars/index.ts caused the dependencies (the zstd node lib required by gcn-kafka) to be required by the main build.

Moving the send function out should resolve the issue, since this will be back, more or less, to a successful build state

I don't think this does that, though. The problem is that we are actually now sending Kafka alerts from the web application. This PR doesn't change that, does it?

[dakota002]

I'm pretty sure that's not the case though, where are you referring to? The only place I see our Kafka send function used is in the table-stream

[lpsinger]

I'm pretty sure that's not the case though, where are you referring to? The only place I see our Kafka send function used is in the table-stream

Right, I understand now. OK, this will work!