-
Notifications
You must be signed in to change notification settings - Fork 190
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
Profile mutation and serialization #2818
Profile mutation and serialization #2818
Conversation
d6826eb
to
0455c8c
Compare
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.
I think ultimately, this isn't going to be the right approach. A full round trip of the file is probably not going to be what users want—I think we will almost certainly want to append to the file instead. That way we can preserve comments, keep the users profile in order, and not worry about unsupported features.
For that reason, I think the thing I'd add serializability for is a profile itself. That also removes the need to make ProfileSet
mutable.
When parsing profiles, we are able to keep track of line number information. If we include that, you could even update your profile by tracking the offsets.
I don't think we'll be able to merge this PR as is unfortunately for the reasons above.
@@ -114,6 +114,11 @@ impl ProfileSet { | |||
self.profiles.get(profile_name) | |||
} | |||
|
|||
/// Set a named profile in the profile set |
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.
let's add docs that clarify that this will replace another profile with the same name. Might as well change the API to return the removed profile if it existed as well
/// Returns a string representation of the profiles, with syntax based on `kind`. | ||
pub fn to_string(&self, kind: ProfileFileKind) -> String { |
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.
I think we'll need a few caveats in this method:
- It is going to delete any comments the user may have had
- We need to test how it handles multi-line values, e.g. for S3:
[profile development]
s3 =
max_concurrent_requests = 20
max_queue_size = 10000
multipart_threshold = 64MB
multipart_chunksize = 16MB
max_bandwidth = 50MB/s
use_accelerate_endpoint = true
addressing_style = path
closing this for now—let us know (or submit a PR) if you'd like to make some more public methods |
Motivation and Context
Supports mutating
ProfileSet
s, so that applications that fetch STS credentials can update aProfileSet
with the new credentials.Supports serializing
ProfileSet
s, so that those credentials can be written back to disk.Description
Testing
New test added to
src/profile/parser.rs
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.