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

sqs dispatcher #718

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

sqs dispatcher #718

wants to merge 2 commits into from

Conversation

ronybony1990
Copy link

send executionPlan and event to sqs queue

@cerveada
Copy link
Contributor

The tests are failing because license header is missing in new files.

@wajda
Copy link
Contributor

wajda commented Jul 10, 2023

@ronybony1990, thank you for the PR, it looks like a really good addition to the Spline agent. However there is one thing I'm unsure of - the dependency on the AWS SDK. It almost doubles the size of the agent bundle and pulls several other transitive dependancies widening the area of potential collision with other agent or spark extensions' dependencies.
Since at the end of the day the SQS request is made through the AWS API call anyway, I'm thinking that maybe calling it directly without SDK library would be a better approach. Error handling and retrial logic doesn't seem too complicated for the use case. Plus there is FallbackLineageDispatcher and CompositeLineageDispatcher that can be used to improve client fault tolerance.
Another option would be making the SDK dependency optional, so that it is not automatically included into the bundle. The user would have to provide all necessary AWS dependencies himself to be able to use SQS dispatcher.
Yet another option would be to create an additional spline-agent-extra-aws module or alike that would contain AWS specific plugins and extensions to the agent, to be used complimentary to the agent-bundle.

@cerveada what do you think?

@cerveada
Copy link
Contributor

I mostly agree.

  • Current solution means big bundle size.
  • Reimplementing the AWS call is more work (not sure how much).
  • optional dependency requires from user to do the dependency resolution.
  • Creating a module is theoretically nice, but it is another part that can cause dependency conflicts.

@sonarcloud
Copy link

sonarcloud bot commented Jul 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

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