👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

test

# export AWS_ENDPOINT_URL=http://10.20.2.192:9000
# export AWS_SECRET_ACCESS_KEY=xxxxx
# export AWS_ACCESS_KEY_ID=xx

# vllm serve s3://llm/meta-llama/Meta-Llama-3-8B-Instruct --load-format runai_streamer
INFO 04-22 11:13:25 [__init__.py:239] Automatically detected platform cuda
....

/cc @DarkLight1337 PTAL.

Is this just a code convention? Or is there official documentation for this?

I'm a little confused why this change is necessary? From my understanding, boto3 should automatically pick up all the variables automatically without having to do this.

Is there any reason why this doesn't work as-is?
cc @chaunceyjiang @DarkLight1337

@vrdn-23 Indeed.

I double-checked the documentation at https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html, and you're right—Boto3 does support those environment variables by default.

I had always assumed it would only read from ~/.aws/config and wouldn't read the environment variables by default. 😂

Is there any reason why this doesn't work as-is?

Yesterday, I set up a MinIO instance myself to test #16926. Then I modified ~/.aws/config for testing.

The minio-py client I commonly use supports those environment variables by default, so in vLLM, I explicitly added the variable as well. 😂

@DarkLight1337 @vrdn-23 For this PR, should we keep the current behavior—explicitly reading the environment variables—or should we revert to the previous behavior—implicitly reading the variables?

I would err on the side of reverting, just because I believe boto3 has an implicit way of deciding which order to read AWS credentials from (eg: it checks env variables first, then client arguments and so on). Explicitly passing in the env variables through the client-side arguments will probably lead to un-intended consequences in the future if that order ever changes.

Let's just revert this then

I would err on the side of reverting, just because I believe boto3 has an implicit way of deciding which order to read AWS credentials from (eg: it checks env variables first, then client arguments and so on). Explicitly passing in the env variables through the client-side arguments will probably lead to un-intended consequences in the future if that order ever changes.

ok.