-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: Pipeline improvements #2
Conversation
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.
PR Type: Enhancement
PR Summary: This pull request introduces several enhancements to the project's CI/CD pipeline and development environment setup. Specifically, it adds steps to the CI workflow for checking type annotations and running Python tests, updates the startup script to conditionally start services based on the last command's exit status, and includes documentation for a sample Glue job based on AWS Glue documentation. These changes aim to improve code quality, security, and developer documentation.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
- Unsupported files: the diff contains files that Sourcery does not currently support during reviews.
General suggestions:
- Given the high importance of the security issue identified with running Jupyter Lab as root, it's crucial to address this by either removing the
--allow-root
flag or ensuring Jupyter Lab runs as a non-root user. - The logic in the startup script's conditional execution based on the last command's exit status should be corrected to accurately reflect the intended behavior, ensuring services start only when previous commands succeed.
- Consider adding more context or examples in the README.md to provide a comprehensive guide for developers using the sample Glue job, enhancing the documentation's usefulness.
- Review the changes to ensure they align with the project's overall security policies, especially concerning the execution permissions and environment configurations.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
start.sh
Outdated
if [[ ! "$?" -eq 1 ]]; then | ||
livy-server start | ||
echo "SSL Disabled" | ||
jupyter lab --no-browser --ip=0.0.0.0 --allow-root --ServerApp.root_dir=/home/glue_user/workspace/ --ServerApp.token='' --ServerApp.password='' |
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.
🚨 security (llm): Hard-coded secrets detected: It appears that the Jupyter server is configured without authentication (empty token and password). This is a significant security risk, especially if this service is exposed to a network. Consider using environment variables or a secure vault solution to manage secrets.
start.sh
Outdated
--ServerApp.root_dir=/home/glue_user/workspace/ \ | ||
--ServerApp.token='' \ | ||
--ServerApp.password='' | ||
if [[ ! "$?" -eq 1 ]]; then |
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.
issue (llm): The condition if [[ ! "$?" -eq 1 ]]; then
checks if the last command was successful, which might not be the intended logic here. This condition will execute the block if the last command failed with an exit code other than 1. If the intention was to ensure the script only proceeds when the previous command was successful, this condition should be revised.
start.sh
Outdated
if [[ ! "$?" -eq 1 ]]; then | ||
livy-server start | ||
echo "SSL Disabled" | ||
jupyter lab --no-browser --ip=0.0.0.0 --allow-root --ServerApp.root_dir=/home/glue_user/workspace/ --ServerApp.token='' --ServerApp.password='' |
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.
🚨 security (llm): The addition of --allow-root
to the Jupyter lab startup command enables Jupyter lab to run as root. This can pose a significant security risk, especially if this script is used in a production environment. Consider running Jupyter lab as a non-root user for better security practices.
5294535
to
1259438
Compare
1259438
to
9d6e6ee
Compare
No description provided.