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
Python/JavaScript: Shared module for serverless functions #13729
base: main
Are you sure you want to change the base?
Conversation
3d9d685
to
dc0282d
Compare
made space for other serverless frameworks in the directory `serverless`
Modelled on the javascript serverless module, but - The predicate that reports YAML files is now public so languages can implement their own file conventions. - It also reports framework and runtime. - The conveninece predicates with files still exist, but they only report the path. - Handler mapping conventions are now documented. - Use parameterised serverless module in Python, tests now pass.
a867c37
to
02c41f3
Compare
Co-authored-by: Rasmus Wriedt Larsen <rasmuswriedtlarsen@gmail.com>
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.
Just some very minor nitpicks, looks very good.
Definitely also looks like something we should consider porting for Ruby - at first glance it doesn't seem like it would take an enormous amount of work to do this.
| * Gets a string suitable as part of a file path. | ||
| */ | ||
| bindingset[base] | ||
| private string normalise(string base) { result = removeLeadingDotSlash(removeTrailingDot(base)) } |
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.
Should be US spelling normalize. Also maybe a more descriptive name, e.g. normalizePath?
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.
Yeah, that is a better name :-)
| if exists(properties.lookup("CodeUri")) | ||
| then codeUri = normalise(lookupValue(properties, "CodeUri")) | ||
| else codeUri = "" |
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.
This "if property exists then lookup the value, else """ pops up a few times in this predicate (also on L108 and L123) - maybe worth factoring out into a little helper predicate.
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.
Good point.
Co-authored-by: Alex Ford <alexrford@users.noreply.github.com>
- rename `normalise` to `normalizePath` - factor out `lookupValueOrEmpty`
Co-authored-by: Alex Ford <alexrford@users.noreply.github.com>
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 - there's one QLDoc failure for Python ServerLess.qll and this is worth adding a changenote for. Otherwise I'm happy.
With this, we find 2 command injections in DVSA.