-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feed download fix #4
Conversation
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 am not familiar with the approach of generating S3 credentials, but it seems sound. I do think we need a lot more comments in general, in the code and config files.
@@ -7,22 +7,22 @@ cache: | |||
directories: | |||
- "$HOME/.m2/repository" | |||
before_install: | |||
- sed -i.bak -e 's|https://nexus.codehaus.org/snapshots/|https://oss.sonatype.org/content/repositories/codehaus-snapshots/|g' | |||
~/.m2/settings.xml | |||
#- sed -i.bak -e 's|https://nexus.codehaus.org/snapshots/|https://oss.sonatype.org/content/repositories/codehaus-snapshots/|g' ~/.m2/settings.xml |
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.
We need comments in this file explaining what the shell commands do.
@zbouziane and @abyrd, I'm going to go ahead and add more comments, fix the conflicts and merge to avoid this branch getting any staler. |
cf3d6f4
to
fd0994b
Compare
Backend changes for #1.
This PR, in conjunction with catalogueglobal/datatools-ui#4, allows for the direct download of feeds from s3 using Temporary Security Credentials (new role in AWS IAM must be created). Rather than using tokens to download feeds (a method which is still used when not using s3), this approach issues credentials to users for the requested feed, which are then used by the client to download the feed from s3.