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

pass around context for plugin actions #365

Open
edaniszewski opened this issue Feb 22, 2019 · 1 comment
Open

pass around context for plugin actions #365

edaniszewski opened this issue Feb 22, 2019 · 1 comment
Assignees

Comments

@edaniszewski
Copy link
Contributor

edaniszewski commented Feb 22, 2019

for read/write/listen behaviors, it would be useful if we can pass in a context so the actual handler function doesn't need to do anything fancy to get at the data it may or may not need. can do action cancellation

@edaniszewski
Copy link
Contributor Author

So. This issue took me down a slight rabbit hole. Initially I was envisioning using the builtin context package, but after reading up on it, I'm almost sure that it is not what I want to do.. at least to solve this issue.

There are a bunch of blog posts out there which have more opinions on this, but the gist of it is that the context, despite its name, is better as a means of cancellation (which is still useful, related: #325) than actually providing contextual values. The solution to that is to not use it as object storage and to just use it as a means of cancellation.

Cancellation is a whole other headache. Cancellation (particularly, write cancellation on write timeout) is what we'd want this for. It can be useful in a small number of cases:

  • the write handler defines retry mechanics
  • the write handler makes http requests which can be cancelled
  • the write handler uses another library which takes a context and handles the cancellation.

The first case.. could happen.. but doesn't really exist right now (I think..) so its marginally useful. The second case doesn't exist right now in any of the plugins. The third case doesn't exist as far as I know. While some libraries could potentially use contexts, the ones we use don't currently, so thats another "case doesn't exist". These cases could exist in the future though, so this still seems like it would be worth implementing.. just not for the initial v3 release.

So right now, we will mark the transaction as failed (timeout), but in reality, the write action will still be running in the background until it eventually either fails or resolves. That is to say, a request timeout can still result in the write happening, which is not something we want, but that is not really a problem that would be solved exclusively with using contexts. Using context cancellation may reduce the number of times that happens (depending on handler implementation), but it won't outright prevent that from happening. In fact, I think this isn't something that the SDK can solve exclusively. It would ultimately be up to the plugin implementer to make sure the handler is written in a safe way that traps all these runaway write cases.

Removing this from the synse v3 project, it can be added later.

@hulksmaaash hulksmaaash added this to the Objective 6 Completion milestone Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants