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

Add a Store implementation for Amazon's S3 #4248

Merged
merged 7 commits into from Dec 16, 2019
Merged

Conversation

@pvalsecc
Copy link
Contributor

pvalsecc commented Dec 3, 2019

The only thing, to my knowledge, that still requires local persistence is thesaurus. But those are OK to be shipped with the code.

This a rebase of #4017 on master.

@fxprunayre fxprunayre mentioned this pull request Dec 3, 2019
5 of 7 tasks complete
@jahow
jahow approved these changes Dec 5, 2019
Copy link
Contributor

jahow left a comment

I've looked at the changes, not much to add but I'm not a Java expert and there are a lot of in-depth changes. Things look much cleaner and streamlined now when it comes to accessing files.

I've also tested the branch with the default file system store and everything seems to work as intended. Thanks @pvalsecc!

Maybe @juanluisrp or @PascalLike can take another look at this?

@fxprunayre fxprunayre added this to the 3.10.0 milestone Dec 6, 2019
@PascalLike

This comment has been minimized.

Copy link
Collaborator

PascalLike commented Dec 6, 2019

Any documentation about the work done?

@fxprunayre

This comment has been minimized.

Copy link
Member

fxprunayre commented Dec 6, 2019

Any documentation about the work done?

This https://github.com/geonetwork/core-geonetwork/pull/4248/files#diff-3fea4016fc5065aab37fe21932c25267R198-R216 is probably the thing to add to the doc indeed ?

@pvalsecc

This comment has been minimized.

Copy link
Contributor Author

pvalsecc commented Dec 9, 2019

Any documentation about the work done?

I would be glad to add some documentation, if you point me where to add it.

@fxprunayre

This comment has been minimized.

Copy link
Member

fxprunayre commented Dec 9, 2019

if you point me where to add it.

Probably around here https://geonetwork-opensource.org/manuals/3.8.x/en/maintainer-guide/installing/customizing-data-directory.html, then click edit on github.

@pvalsecc

This comment has been minimized.

Copy link
Contributor Author

pvalsecc commented Dec 9, 2019

Thanks @fxprunayre, PR created

@jahow jahow merged commit 6c80a91 into geonetwork:master Dec 16, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pvalsecc pvalsecc deleted the pvalsecc:s3_master branch Dec 16, 2019
@fxprunayre

This comment has been minimized.

The source image is not in appDir, so we should replace this by

            try(ResourceHolder src = getImage(context, srcPath.getFileName().toString(), srcPath.getParent());

Right ?

This comment has been minimized.

Copy link
Contributor Author

pvalsecc replied Jan 9, 2020

I don't understand why I'd say that it's not in app dir. Before it was doing a locateResource with appDir. So, unless the previous version was wrong, I'd say the new version is correct.

This comment has been minimized.

Copy link
Member

fxprunayre replied Jan 10, 2020

locateResource is the same, but getImage is looking in the wrong place. Will fix this.

fxprunayre added a commit to fxprunayre/core-geonetwork that referenced this pull request Jan 10, 2020
fxprunayre added a commit that referenced this pull request Jan 10, 2020
harvester.

Related to #4248
cmangeat added a commit to geoadmin/geocat that referenced this pull request Jan 10, 2020
@@ -474,8 +479,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
dataManager.activateWorkflowIfConfigured(context, newId, group);

try {
copyDataDir(context, sourceMetadata.getId(), newId, Params.Access.PUBLIC);
copyDataDir(context, sourceMetadata.getId(), newId, Params.Access.PRIVATE);
StoreUtils.copyDataDir(context, sourceMetadata.getId(), Integer.parseInt(newId), true);

This comment has been minimized.

Copy link
@fxprunayre

fxprunayre Jan 17, 2020

Member

Creating a record with an Editor using a template that the user can not download fails with
{"message":"SecurityException","code":"forbidden","description":"Current user can't download resources for metadata 'e24b89cc-2e66-49e7-84b6-19f1ab106128' and as such can't access the requested resource."}

StoreUtils.copyDataDir may return SecurityException now and creation fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.