Skip to content
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

Auth and run steps #19

Merged
merged 5 commits into from
Dec 22, 2024
Merged

Auth and run steps #19

merged 5 commits into from
Dec 22, 2024

Conversation

kevinschweikert
Copy link
Collaborator

Supersedes #14

This builds upon #14 to add more auto options and at the same time some finetuning options which req steps are run before generating the curl command

@kevinschweikert kevinschweikert marked this pull request as draft December 4, 2024 21:34
@kevinschweikert kevinschweikert marked this pull request as ready for review December 4, 2024 21:40
lib/curl_req.ex Outdated Show resolved Hide resolved
@kevinschweikert
Copy link
Collaborator Author

i will add some more thest to verify all the auth options

@kevinschweikert
Copy link
Collaborator Author

Hey @derekkraan, this should be ready now! I've added netrc/more auth handling to generation and parsing and refined the functionality to define which steps to run but i'm still not sure if we should implement that. What is your opinion? The only current benefit would be, that req would not try to read in the netrc file and raise if not found. Then you would need to skip the :auth step

@derekkraan
Copy link
Owner

derekkraan commented Dec 22, 2024

@kevinschweikert Is the :only and :except options needed to support a concrete use-case? Is your scenario that the --netrc option is given (in a copied curl command), but that you can override this by passing except: [:auth]?

In that case, I think it's fine to add it. If I am misunderstanding, then please let me know.

Can you add @acalejos as a co-author to the relevant commits that he helped to author? GitHub has a guide on how to do that.

@derekkraan
Copy link
Owner

And we should also record this in the changelog.

@kevinschweikert
Copy link
Collaborator Author

@derekkraan thanks for the feedback. The use case is exactly how you described. The auth step would try to read in the netrc file and raises when it does not exists. We wouldn't need this to run, because we get the filepath from the Req.Request options and let cURL read in the file at execution time.

Will clean up the commits and add the co-author...must have lost it in a rebase. Thanks for the hint!

kevinschweikert and others added 2 commits December 22, 2024 11:53
Co-authored-by: Andres Alejos <andres.c.alejos.mil@army.mil>
Co-authored-by: Andres Alejos <andres.c.alejos.mil@army.mil>
kevinschweikert and others added 2 commits December 22, 2024 12:07
Co-authored-by: Andres Alejos <andres.c.alejos.mil@army.mil>
@kevinschweikert
Copy link
Collaborator Author

@derekkraan this should be ready now!

@derekkraan derekkraan merged commit 9cb678e into main Dec 22, 2024
6 checks passed
@derekkraan derekkraan deleted the feature/more-auth branch December 22, 2024 12:37
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.

2 participants