Wpb 27230 add preflight - #5
Conversation
Veki301
left a comment
Veki301
left a comment
There was a problem hiding this comment.
standard question was it tested :D ?
…macs, and shorten the target names.
…ntrypoint differently.
sghosh23
left a comment
sghosh23
left a comment
There was a problem hiding this comment.
The PR handles multiple responsibilities. Based on my understanding just by going through the changes:
- Pref-light checks
- Package restructure
- Multi-mode CLI dispatcher
- version reporting
- Dockerfile
- TCP probe and DNS utilization
At this point reviewing this PR is not easy and there are lot of questions which i did not ask as i am lacking the context.
First thing first: Is this the right tool for per-flight check. This utility tool has only one specific job.
|
|
||
| # Warning: Version restriction here. | ||
| RUN apt-get remove rabbitmq-server=3.13.6-1 -y | ||
|
|
There was a problem hiding this comment.
What is the motivation of changing the rabbitmq client configurations?
Feels a complicated process to maintain unless there is a solid reason and if so this can be refactored to a script with proper documentation rather significantly large Dockerfile.
| CASSANDRA_SERVICE_NAME = os.getenv('CASSANDRA_SERVICE_NAME', '') | ||
| CASSANDRA_SERVICE_PORT = int(os.getenv('CASSANDRA_SERVICE_PORT')) if os.getenv('CASSANDRA_SERVICE_PORT') else None | ||
|
|
||
| # only import these if there is a cassandra to test. |
There was a problem hiding this comment.
is this for the future that one day there wont be any cassandra?
If that's the case I would group the ENV vars together
| ).stdout | ||
| data = json.loads(out) if out else {} | ||
| if "version" in data: | ||
| versions["minio"] = data["version"] |
There was a problem hiding this comment.
Does the data contain the version? Please check the endpoint doc: https://docs.min.io/aistor/operations/monitoring/healthcheck-probe/
There was a problem hiding this comment.
mc admin info might give you what you are looking for!
| elif args.command == "status": | ||
| # Fast path – the Bash alias `status` already prints a table. | ||
| # We simply exit with success so the container can be used as a one‑shot check. | ||
| sys.exit(0) |
There was a problem hiding this comment.
then i dont understand the point of adding it here
| cass_versions[f"{ip}:{CASSANDRA_SERVICE_PORT}"] = ver | ||
| if cass_versions: | ||
| versions["cassandra"] = cass_versions | ||
|
|
There was a problem hiding this comment.
prints
"cassandra": {
"192.168.122.31:9042": "-----------------",
"192.168.122.32:9042": "-----------------",
"192.168.122.33:9042": "-----------------"
},
3 participants
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
Briefly describe the issue you have solved or implemented with this pull request. If the PR contains multiple issues, use a bullet list.
Causes (Optional)
Briefly describe the causes behind the issues. This could be helpful to understand the adopted solutions behind some nasty bugs or complex issues.
Solutions
Briefly describe the solutions you have implemented for the issues explained above.
Dependencies (Optional)
If there are some other pull requests related to this one (e.g. new releases of frameworks), specify them here.
Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
Attachments like images, videos, etc. (drag and drop in the text box)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.