Add recommendations backend boilerplate#4301
Add recommendations backend boilerplate#4301akolson merged 8 commits intolearningequality:unstablefrom
Conversation
| class Recommendations(Backend): | ||
|
|
||
| def connect(self) -> None: | ||
| return super().connect() |
There was a problem hiding this comment.
I was just curious, assuming that we would be needing this connect method we might want to check in other functions that request, and make_request if the connection is currently alive? if its not then we may want to establish the connection first? I believe this should not be needed in the class code itself and should be handled by the developer while using the class but a safety check before making requests can be useful?
There was a problem hiding this comment.
Yes @ozer550 you raise a great point. I am fine with either removing it or it remaining. Though the latter enforces what needs to be done and rids the make_request from any connection detail(Single responsibility pattern!!). I am almost certain all backends have some form of connection. Its very unlikely that they keep their door open to you all the time 🙂
There was a problem hiding this comment.
yes, this makes sense! I think we should let it remain here.
| class Recommendations(Backend): | ||
|
|
||
| def connect(self) -> None: | ||
| return super().connect() |
There was a problem hiding this comment.
Yes @ozer550 you raise a great point. I am fine with either removing it or it remaining. Though the latter enforces what needs to be done and rids the make_request from any connection detail(Single responsibility pattern!!). I am almost certain all backends have some form of connection. Its very unlikely that they keep their door open to you all the time 🙂
2 participants
Summary
Description of the change(s) you made
This PR adds
Recommendationsbackend with its boilerplate.Manual verification steps performed
Not applicable under the circumstances
Does this introduce any tech-debt items?
Reviewer guidance
How can a reviewer test these changes?
References
Closes #4296
Comments
Contributor's Checklist
PR process:
CHANGELOGlabel been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocslabel has been added if this introduces a change that needs to be updated in the user docs?requirements.txtfiles also included in this PRStudio-specifc:
notranslateclass been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages,components, andlayoutsdirectories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarnandpip)