-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
improve: generalize transformations and scripts of runner and preloads into TemplateTransformer #4487
Conversation
d0f17bb
to
79d03d5
Compare
e1d4b8b
to
5153b54
Compare
3b5dfe5
to
99f5c94
Compare
d66cd94
to
05607f3
Compare
BTW, during the refactoring, it's found that the approaches for the input objects serialization vary between Python and Javascript. The one for Python is wrapped in Base64 encoding, while the Javascript uses the raw inputs json object. Not the best way for further extensions to other languages. Is that as designed on purpose? |
In Python, the boolean value True is capitalized, which is not standard in JSON encoding. As a result, simple JSON encoding might not work as unexpected. However, this issue does not occur in JavaScript. |
Yes, the differences were noticed. So maybe we could unify the json+base64 as the standard serialization for input objects in the future. |
And there's another point I may want to have your suggestions. We are still using |
feel free to do it~ |
It's unnecessary I think, the |
Yes, I agree with you. Let's keep it with "replace". |
Hi, I have get this done. |
👍
W. Logan Clark
…On Sun, May 19, 2024 at 1:49 AM Bowen Liang ***@***.***> wrote:
Yes, the differences were noticed. So maybe we could unify the json+base64
as the standard serialization for input objects in the future.
feel free to do it~
Hi, I have get this done. json + base64 serialization is now a shared
mechanism for both Python and Javascript.
—
Reply to this email directly, view it on GitHub
<#4487 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BH6TQ24YBKSFBUTF2TPZOOTZDBRS7AVCNFSM6AAAAABH4CLMXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZGE2TMMZZGQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
cac92f6
to
bd4a5a1
Compare
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.
LGTM
Description
dedent
inside methodsType of Change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Suggested Checklist:
dev/reformat
(backend) andcd web && npx lint-staged
(frontend) to appease the lint godsoptional
I have made corresponding changes to the documentationoptional
I have added tests that prove my fix is effective or that my feature worksoptional
New and existing unit tests pass locally with my changes