-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Docker Shells configuration #3295
Docker Shells configuration #3295
Conversation
WalkthroughThe changes centralize the Changes
Suggested labels
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docker/api.Containerfile
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (3)
docker/api.Containerfile (3)
35-39
: Well-structured shell initialization setup!Good implementation of the global profile script following best practices:
- Correct location in
/etc/profile.d/
- Proper executable permissions
- Comprehensive
fnm
initialization with all necessary flags
41-41
: Excellent non-interactive shell configuration!Setting
BASH_ENV
ensures consistentfnm
availability in non-interactive bash shells, which is crucial for CI/CD pipelines and scripts.
43-43
: Good shell coverage with explicit sourcing!Adding explicit sourcing in
.bashrc
ensures thefnm
environment is consistently available across all shell types (login, non-interactive, and interactive).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3295 +/- ##
=================================================
Coverage 48.36% 48.36%
=================================================
Files 458 458
Lines 34516 34516
Branches 966 966
=================================================
Hits 16695 16695
Misses 17821 17821 ☔ View full report in Codecov by Sentry. |
…a-api into docker-shell
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.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docker/api.Containerfile
(1 hunks)docs/docs/docs/getting-started/installation.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/docs/docs/getting-started/installation.md
396-396: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (2)
docker/api.Containerfile (2)
35-43
: LGTM! Well-structured shell environment configuration.The changes effectively centralize the fnm configuration by:
- Creating a global profile script that loads in both login and non-interactive shells
- Setting BASH_ENV to ensure non-interactive shells load the configuration
- Explicitly sourcing in .bashrc for login shells
46-46
: LGTM! Secure curl command implementation.The curl command properly implements security best practices by using:
- HTTPS-only connections (
--proto '=https'
)- Modern TLS version (
--tlsv1.2
)
@palisadoes PR is ready for review |
b8d4a95
into
PalisadoesFoundation:develop-postgres
Related issue #3214
Description
Configure docker login shells and bash non interactive shells to use fnm.
Conclusion: Bash can we widely used as per needs.
Following can be used to at ease now as one line commands directly from cli:
docker exec talawa-api-1 /bin/bash -c 'pnpm run add:sample_data && exit'
docker exec talawa-api-1 /bin/bash -l -c 'pnpm run add:sample_data && exit'
docker exec talawa-api-1 /bin/sh -l -c 'pnpm run add:sample_data && exit'
docker exec -it talawa-api-1 /bin/bash -i -c 'pnpm run add:sample_data && exit'
docker exec -it talawa-api-1 /bin/bash -l -i -c 'pnpm run add:sample_data && exit'
docker exec -it talawa-api-1 /bin/sh -l -i -c 'pnpm run add:sample_data && exit'
Key Changes
Using reset:data script (interactive)
docker exec -it talawa-api-1 /bin/bash -c 'pnpm run reset:data'
Next Steps For issue
Improve code coverage for helpers script.
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Chores