-
Notifications
You must be signed in to change notification settings - Fork 26
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
NoAuthRequest Test Fix #92
Comments
bors bot
added a commit
that referenced
this issue
Oct 24, 2019
88: Migrate AWSAuth.jl into AWSCore.jl r=iamed2 a=mattBrzezinski Overview --- This merge request is part of a larger overall refactor. Recently this merge request went through in [HTTP.jl](JuliaWeb/HTTP.jl#443), it introduced the ability to add custom layers into the request stack. With that merge request being introduced we can now move the `AWS4AuthLayer` away from the `HTTP.jl` project. We need to find a new home for `AWSAuthLayer`, at first glance it makes most sense to move this into the `AWSAuth.jl` package. However, looking at the state of the package, and open issues we can see why this might not be the best plan. `AWSAuth.jl` has an open [issue](JuliaCloud/AWSAuth.jl#11) to move `AWSCredentials` from `AWSCore.jl` into it. However, if we did this we would introduce a circular dependency between `AWSAuth.jl` and `AWSCore.jl`. To get around this circular dependency we would need to move [Services.jl](https://github.com/JuliaCloud/AWSCore.jl/blob/master/src/Services.jl) into its own package. I do not think this is the best decision as we are creating more packages and not solving the core issue with the state of these packages. `AWSAuth.jl` is a tiny package, which is just a duplicate of the `AWS4AuthLayer` in `HTTP.jl`. I propose that we move this entire functionality into `AWSCore.jl`, and archive `AWSAuth.jl`. The addition of this is not a heavy lift into `AWSCore.jl` and helps centralize these AWS packages. I think this is a good step forward and will help clean up the current state of the JuliaCloud packages. Afterwards we can start looking ahead and re-designing the architecture behind these packages. Next Steps --- If this merge request is approved, we can clean up `HTTP.jl` and remove the `AWS4AuthLayer` code from that package as it already exists in this one. We can then begin looking into refactoring this package and making it more modular. [AWSAuth.jl](https://github.com/JuliaCloud/AWSAuth.jl) can be archived as it no longer has a purpose. Links --- - [AWSAuth.jl - Issue - Current state of the package](JuliaCloud/AWSAuth.jl#11) - [HTTP.jl - CR - Add Custom Layers to request stack](JuliaWeb/HTTP.jl#443) - [HTTP.jl - Issue - Move AWS4AuthLayer](JuliaWeb/HTTP.jl#355) Issues deemed OOO that are logged: --- - #90 - #91 - #92 Co-authored-by: Matt Brzezinski <[email protected]>
bors bot
added a commit
that referenced
this issue
Oct 28, 2019
88: Migrate AWSAuth.jl into AWSCore.jl r=mattBrzezinski a=mattBrzezinski Overview --- This merge request is part of a larger overall refactor. Recently this merge request went through in [HTTP.jl](JuliaWeb/HTTP.jl#443), it introduced the ability to add custom layers into the request stack. With that merge request being introduced we can now move the `AWS4AuthLayer` away from the `HTTP.jl` project. We need to find a new home for `AWSAuthLayer`, at first glance it makes most sense to move this into the `AWSAuth.jl` package. However, looking at the state of the package, and open issues we can see why this might not be the best plan. `AWSAuth.jl` has an open [issue](JuliaCloud/AWSAuth.jl#11) to move `AWSCredentials` from `AWSCore.jl` into it. However, if we did this we would introduce a circular dependency between `AWSAuth.jl` and `AWSCore.jl`. To get around this circular dependency we would need to move [Services.jl](https://github.com/JuliaCloud/AWSCore.jl/blob/master/src/Services.jl) into its own package. I do not think this is the best decision as we are creating more packages and not solving the core issue with the state of these packages. `AWSAuth.jl` is a tiny package, which is just a duplicate of the `AWS4AuthLayer` in `HTTP.jl`. I propose that we move this entire functionality into `AWSCore.jl`, and archive `AWSAuth.jl`. The addition of this is not a heavy lift into `AWSCore.jl` and helps centralize these AWS packages. I think this is a good step forward and will help clean up the current state of the JuliaCloud packages. Afterwards we can start looking ahead and re-designing the architecture behind these packages. Next Steps --- If this merge request is approved, we can clean up `HTTP.jl` and remove the `AWS4AuthLayer` code from that package as it already exists in this one. We can then begin looking into refactoring this package and making it more modular. [AWSAuth.jl](https://github.com/JuliaCloud/AWSAuth.jl) can be archived as it no longer has a purpose. Links --- - [AWSAuth.jl - Issue - Current state of the package](JuliaCloud/AWSAuth.jl#11) - [HTTP.jl - CR - Add Custom Layers to request stack](JuliaWeb/HTTP.jl#443) - [HTTP.jl - Issue - Move AWS4AuthLayer](JuliaWeb/HTTP.jl#355) Issues deemed OOO that are logged: --- - #90 - #91 - #92 Co-authored-by: Matt Brzezinski <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Not to get too far out-of-scope with #88 but these tests run deterministically of the files existing or not. It seems that the intended behaviour was if these files exist, ensure the proper response was given back. If an error is thrown make sure it's either a
AccessDenied
(the file is not private) orEntityDoesNotExist
(the file was removed).These tests should generate their own public files, and ensure that it can access them.
The text was updated successfully, but these errors were encountered: