Comments (14)
Sorry for the delay in replying. Currently trying to get over a horrible cold.
There will always be differences in my packages compared to the official SDKs, I'm not a copy/paste kinda person - but you're spot on with the need for some way of the persistence store being able to handle the uniqueness of each request.
If you want to submit a PR, feel free - that isn't going to break the RequestHandlers repo. Anything that changes the contract will be a breaking change, and therefore a major version bump, so it won't get picked up by the dependent repos. That's why I don't need to worry about merging the repos, if I version properly it shouldn't cause trouble.
I'll be looking at fixing this in both repos moving forward anyway - now you've highlighted it I don't know how I missed it to start with 😄 Thank you!
from alexa.net.statemanagement.
I know you said that you aren't a copy/paste kinda person, but I do feel like there is real value in having a separate Save method for the IPersistence interface. Especially because you are setting records one at a time, I don't want to have to implement saving every single time a value is set. I'd rather set a collection and save all at once. If you add a Save method to the interface, I would at least have the option. I understand that not everyone will implement this the same way I have, and may prefer to save on each Set, but having the Save method gives the end user the option of implementing either way.
Just my $.02
from alexa.net.statemanagement.
@ncipollina I agree it's down to each store to decide how to save data, so why should it be forced for all?
There's nothing stopping your implementation having a final save method that is called - the interface just ensures it will work within the rest of the package structures. What am I missing?
from alexa.net.statemanagement.
I'm just suggesting you add a Save method to the interface and to the ISkillState interface. If you don't want to use that method in your implementation, you just don't add the saving logic to the method. Below is an example of how I envision this interface looking:
public interface IPersistenceStore
{
Task<T> GetAsync<T>(SkillRequest request, string key);
Task SetAsync<T>(SkillRequest request, string key, T value);
Task SaveAsync(SkillRequest request);
}
and
public interface ISkillState
{
Task<T> Get<T>(string key);
T GetRequest<T>(string key);
T GetSession<T>(string key);
Task<T> GetPersistent<T>(string key);
void SetRequest<T>(string key, T value);
void SetSession<T>(string key, T value);
Task SetPersistent<T>(string key, T value);
Session Session { get; }
Task SavePersistent();
}
Otherwise, if I want to implement the Saving logic separately from the Setting logic, I have to implement my own ISkillState instance rather than using the SkillState implementation provided in your package.
from alexa.net.statemanagement.
Sorry @ncipollina - not trying to be difficult, I understood the logic you were asking for - I just needed a solid use case explaining why this is good for all implementations. The SkillRequest change you requested was spot on - very difficult to generate any persistent store that can work alongside lots of requests without that information being available in the interface. Adding it makes everything clearer and cleaner. That's why it's already changed 👍
To use the SkillState object though, you have to pass in the IPersistenceStore object, or set it against the Pipeline if you're using the RequestHandler package.
If you look at the following examples below, they seem to give the same result - but don't force this possibly optional method into the implementation. Yes, they're simplistic for examples in this thread, but the point is to show that saving the store isn't an integral part of the SkillState object - it's code specific and therefore not required in the interface. These save methods could just as well be part of ASP.NET Core MiddleWare or DI life cycle maintenance etc.
var skillState = new SkillState(request, myPersistenceStore);
//...skillState logic...
await myPersistenceStore.Save(request);
or, if you're using RequestHandlers
var conversation = new AlexaRequestPipeline();
//...pipeline setup....
conversation.StatePersistence = myPersistenceStore;
await conversation.Process(skillRequest)
await myPersistenceStore.Save(skillRequest);
Appreciate you talking this through - it's hard when I have implementations that would definitely use this. But it still feels like an optional step, and therefore not right for the interface.
from alexa.net.statemanagement.
@stoiveyp, I too am not trying to be difficult. I could definitely go with the solution you suggested, my only issue is that in my RequestHandler, I have a reference to an IPersistenceStore object and not the actual implementation type (I'm using dependency injection). Since the type is IPersistenceStore, the compiler won't know about my Save method, and the example you provided will not work.
from alexa.net.statemanagement.
That's great, thanks! Now I have a use case I can look at a suitable alternative.
Got an idea - I'll take a proper look after work tonight (few hours) and add you as a reviewer.
from alexa.net.statemanagement.
@ncipollina while I'm in my meeting - could this not be handled by inheriting IPersistenceStore and using that interface for DI?
public ISaveablePersistenceStore
: IPersistenceStore
{
Task Save(SkillRequest request);
}
Apologies if the formatting is off - using mobile app
from alexa.net.statemanagement.
Adding this to the package would be an easy minor version bump - totally doable. Thoughts?
from alexa.net.statemanagement.
@stoiveyp this seems like a good approach. You'd need to update the SkillState class to use this interface, but this would be a good compromise. For folks that don't want to have a separate Save method, they could use the IPersistenceStore interface and for those that want save, they could use ISaveablePersistenceStore. Seems like a good compromise.
from alexa.net.statemanagement.
Why would SkillState need it if, in your use case, you can access it via the interface?
from alexa.net.statemanagement.
I'm interfacing with the Persistence through the SkillState instance. I guess my idea of how to implement this just differs from your vision.
from alexa.net.statemanagement.
I do feel we're entering the realm of customising the library specifically for your use case I'm afraid. Casting the SkillState store to ISavablePersistenceStore and running the method would work in your scenario.
SkillState is there to allow code to access the different levels of state in a single way, and RequestHandlers are there to easily alter logic within the skill without changing the flow in and out of the hosting code.
Neither package are built to handle user specific data life cycle management, which is why they're not in the interfaces.
The overriden interface is probably your best bet for handling this, but I appreciate you raising the issue, and I was happy to add the SkillRequest code to the persistence stores - thanks for the conversation :slightly_smiling:
Equally happy to carry on the conversation from a tech view outside of the library context - SkillState requires per request knowledge so it sounds like really interesting DI setup.
from alexa.net.statemanagement.
Yeah, I'd say let's leave well enough alone.
from alexa.net.statemanagement.
Related Issues (5)
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from alexa.net.statemanagement.