Skip to content

Conversation

@yashsinghcodes
Copy link
Member

If we do something like print([str(i) for i in range(10)])

this returns a list which looks like this:
['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']

Now the problem is if we do json.loads() to this string (this is treaded as str in app_sdk), it throws an exception because the input string like ['1','2'] is not valid JSON — it's a Python-style list, not JSON-compliant.

Hence a case where we can handle such problem and fix it for the user. Now this exist only in OnPrem not sure how but cloud setup automatically converts this: '1' to this: "1" (probably frontend?).

@frikky
Copy link
Member

frikky commented May 20, 2025

If we do something like print([str(i) for i in range(10)])

this returns a list which looks like this: ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']

Now the problem is if we do json.loads() to this string (this is treaded as str in app_sdk), it throws an exception because the input string like ['1','2'] is not valid JSON — it's a Python-style list, not JSON-compliant.

Hence a case where we can handle such problem and fix it for the user. Now this exist only in OnPrem not sure how but cloud setup automatically converts this: '1' to this: "1" (probably frontend?).

Are you sure this handles this exact case we are dealing with now and not any case? This seems like it could definitely break something as it's trying to autoparse stuff that may not be parsable, then forcing it into a JSON object

Frontend doesn't translate anything. Please find out why exactly it only happens onprem, as this is not an onprem problem. It should be a generic one, or none at all.

If anywhere, it has to be in the App SDK or Worker/cloud backend I presume.

PS: Isn't this related to the true -> True, false -> False etc. issue that our customer had recently? Weird formatting.

@yashsinghcodes
Copy link
Member Author

Hey @frikky, sorry for the confusion but just tested this onCloud as well and like it does exist in there as well here is the execute python code that help me reproduce it: print([str(i) for i in range(10)]).

Are you sure this handles this exact case we are dealing with now and not any case? This seems like it could definitely break something as it's trying to autoparse stuff that may not be parsable, then forcing it into a JSON object

This does handle the exact case, not sure any other condition where it can actual break something if we hit this exception, but I know app_sdk needs to be well put together so if you want I can make it more specific?

PS: Isn't this related to the true -> True, false -> False etc. issue that our customer had recently? Weird formatting.

I don't think it solves that, this look more like us enforce python boolean on "true" and "false" value have to look into it.

@frikky
Copy link
Member

frikky commented May 21, 2025

Hey @frikky, sorry for the confusion but just tested this onCloud as well and like it does exist in there as well here is the execute python code that help me reproduce it: print([str(i) for i in range(10)]).

Are you sure this handles this exact case we are dealing with now and not any case? This seems like it could definitely break something as it's trying to autoparse stuff that may not be parsable, then forcing it into a JSON object

This does handle the exact case, not sure any other condition where it can actual break something if we hit this exception, but I know app_sdk needs to be well put together so if you want I can make it more specific?

PS: Isn't this related to the true -> True, false -> False etc. issue that our customer had recently? Weird formatting.

I don't think it solves that, this look more like us enforce python boolean on "true" and "false" value have to look into it.

I do wonder what the general way of handling this should be tbh. Let's merge and try it.

Do we have any tests in the test workflow for these kinds of cases?

@frikky frikky merged commit 37ba184 into Shuffle:main May 21, 2025
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.

2 participants