-
-
Notifications
You must be signed in to change notification settings - Fork 493
Fixed Error invoking remote method 'setStoreValue': TypeError: Use delete() to clear values #887
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
Conversation
…lete() to clear values
deep1401
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to reproduce the original error on the main branch by deleting an experiment in app and also in the cloud mode but I wasn't able to reproduce the error. Not sure if it is windows-specific?
Are you able to see it in the cloud mode? (npm run start:cloud)
I got this on the desktop app. Maybe I can send a video of the error on discord ? |
|
I'm OK with adding the check for valid experiments in the app, but I would prefer if we did the filter at the API/SDK level. i.e. inside the function that gets a list of experiments in @deep1401 Tagging you in case you have input on if we should filter experiments that are missing experiment_id or experiment_name from their json file? (Param had an experiment that was somehow missing both in its json...no idea how he made it!) Or do you think we should just handle it by adding the ID when listing or something? |
dadmobile
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in comment...requesting we either add a check for this in SDK (or if we decide these should be handled then make SDK work in this case).
I thought I solved this by having a condition in the sdk where if an experiment has a name but no id then it should just use the same name as the id. I'm not sure how to replicate this case but we should for sure do it in the sdk itself in the function which tries to list all experiments. |
|
Yeah. Got it. I will make a Fix on the sdk instead of on the UI 🫡 |
…ab-app into fix/delete_values
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
This sort of breaks a rule about side effects on get calls (generally try to avoid doing writes/updates etc. inside of a call that you don't expect to change the system). But I guess it's OK because it fixes a thing and should be faster going forward. But I don't understand why the build is failing like that. Is it because you are changing the one pyproject.toml? |
Fixes #876