Skip to content

Conversation

leezu
Copy link
Contributor

@leezu leezu commented Jun 23, 2025

This extends the previous AWS_PROFILE based get-credentials implementation to allow explicit setting of the profile

This extends the previous AWS_PROFILE based get-credentials
implementation to allow explicit setting of the profile
@karthink
Copy link
Owner

karthink commented Jul 9, 2025

Pinging @akssri, @felipeochoa: do you have any comments about this PR?

@felipeochoa
Copy link

Is the idea that you might have different gptel instances running different profiles? Or what does this buy over just calling setenv?

@akssri
Copy link
Contributor

akssri commented Jul 9, 2025 via email

@leezu
Copy link
Contributor Author

leezu commented Jul 10, 2025

@akssri added a commit to swap the precedence of argument and environment variable.

@felipeochoa users may wish to use a separate aws profile for gptel compared to other activities triggered from emacs. setenv would set the environment variable for all processes inheriting the emacs environment variables.

Copy link
Contributor

@akssri akssri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lgfang
Copy link

lgfang commented Aug 4, 2025

Hello, I noticed this pull request after submitting my own, (#996). Both PRs address the same goal of adding an optional :profile argument to gptel-make-bedrock, but they do so with a couple of key differences.

Comparison of Implementations

  1. Handling of the :profile Parameter

    In my opinion, the approach in my PR is a bit more concise as it avoids passing the parameter through intermediate functions.

  2. Credential Lookup Order

    The order in which AWS credentials are sourced is also different. This can affect which credentials are used if multiple sources are configured.

    • This PR's Order:

      1. The AWS_ACCESS_KEY_ID and related environment variables
      2. The :profile value provided in the backend configuration.
      3. The AWS_PROFILE environment variable.
    • My PR's (gptel-bedrock: allow configuring AWS profile via gptel-make-bedrock #996) Order:

      1. The :profile value provided in the backend configuration.
      2. The AWS_ACCESS_KEY_ID and related environment variables
      3. The AWS_PROFILE environment variable.

    My implementation prioritizes the explicitly configured :profile over the general environment variables, which might be a more expected behavior for users who set a specific profile for gptel. The description in my PR contains the detailed reasoning for this credential order.

I'm happy to discuss these differences and consolidate our efforts. Thank you for your work on this

@akssri akssri mentioned this pull request Aug 30, 2025
@akssri
Copy link
Contributor

akssri commented Aug 30, 2025

(Moved to #1053)

@karthink
Copy link
Owner

Closing in favor of #1053.

@karthink karthink closed this Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants