Skip to content

Conversation

@jtanx
Copy link

@jtanx jtanx commented Sep 27, 2025

Profiles should be read out of the config file, not credentials file

I believe this fixes #111

@Tmonster
Copy link
Collaborator

Hi @jtanx , Thanks for the PR!

Can you rebase and merge with the v1.4-andium branch and target that as your base branch? That should fix the build error.

@jtanx jtanx changed the base branch from main to v1.4-andium September 30, 2025 13:25
@jtanx
Copy link
Author

jtanx commented Sep 30, 2025

ah yep, done

jtanx added 2 commits October 4, 2025 00:26
Profiles should be read out of the config file, not credentials file
@jtanx
Copy link
Author

jtanx commented Oct 6, 2025

Are there instructions for running the minio tests locally?

@jtanx
Copy link
Author

jtanx commented Oct 8, 2025

Alright I just ran the actions on my fork and the tests work now.

@jtanx
Copy link
Author

jtanx commented Oct 15, 2025

@Tmonster any chance for another review?

@nrsmac
Copy link

nrsmac commented Nov 3, 2025

@Tmonster +1!

Copy link
Collaborator

@Tmonster Tmonster left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

// get the profile from within that file
Aws::Map<Aws::String, Aws::Config::Profile> profiles;
Aws::Config::AWSConfigFileProfileConfigLoader loader(credentials_file_path);
Aws::Config::AWSConfigFileProfileConfigLoader loader(config_file_path, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe in the future we will want a setting for the useProfilePrefix, but don't think that is needed here

@jtanx
Copy link
Author

jtanx commented Nov 4, 2025

Nice, thanks for reviewing; do I also need someone else to review to be able to merge?

@Tmonster
Copy link
Collaborator

Tmonster commented Nov 5, 2025

nope, thanks for the PR!

@Tmonster Tmonster merged commit 55bf362 into duckdb:v1.4-andium Nov 5, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Profile loading is reading the credentials file instead of the config file

4 participants