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

Setting the download directory so you only get one instance downloaded… #1081

Closed
wants to merge 1 commit into from
Closed

Conversation

carolyncole
Copy link
Contributor

@carolyncole carolyncole commented May 17, 2016

per machine. fixes #1080

@@ -2,3 +2,4 @@
port: 8984
enable_jms: false
fcrepo_home_dir: tmp/fcrepo4-development-data
download_dir: /tmp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these should be <%= ENV['TMPDIR'] %>? @cbeer ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd certainly be better (and cross platform..)... although isn't this the same as the default behavior upstream?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<%= ENV.fetch('TMPDIR', '/tmp') %> ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbeer Yes, that is curious. Why is it downloading then?

@jcoyne
Copy link
Member

jcoyne commented May 17, 2016

@Cam156 are you sure this fixes the issue?

Prior to using this change I see:

require 'solr_wrapper'
=> true
irb(main):002:0> SolrWrapper.instance(config: 'config/solr_wrapper_test.yml').config.instance_dir
=> "tmp/solr-test"
irb(main):003:0> SolrWrapper.instance(config: 'config/solr_wrapper_test.yml').config.download_dir
=> "/var/folders/9t/rygbnddx0b1ckw6tjs3m18qm0000gq/T"

So it appears as though it's already looking in the tmp directory for the download_dir.

@carolyncole
Copy link
Contributor Author

carolyncole commented May 17, 2016

@jcoyne looking at the default here: https://github.com/cbeer/solr_wrapper/blob/master/lib/solr_wrapper/configuration.rb#L58-L60
If you are in a rails app it uses tmp instead of /tmp
So depending on where you run the wrapper it behaves differently. So I am not 100% sure, but I would believe it to work...

@jcoyne
Copy link
Member

jcoyne commented May 17, 2016

@Cam156 Yes, I see that. I wonder if it makes more sense to just remove that logic from solr_wrapper?

@carolyncole
Copy link
Contributor Author

@jcoyne I would be ok with that approach.

@mjgiarlo
Copy link
Member

@Cam156 @jcoyne so am I hearing that this will be fixed in solr_wrapper, and no related change is needed in AF? or?

@jcoyne
Copy link
Member

jcoyne commented May 23, 2016

I think this should get fixed in solr_wrapper. It's a broader issue than just AF.

@mjgiarlo
Copy link
Member

OK. @Cam156 should we close this issue?

@carolyncole
Copy link
Contributor Author

@mjgiarlo Is there an issue on Solr wrapper? I would leave it open until there is...

@mjgiarlo
Copy link
Member

@Cam156 There are 0 open issues on solr_wrapper, so no.

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.

setting instance_dir causes solr to download for each project
6 participants