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

Removing gemini #822

Merged
merged 13 commits into from
Mar 12, 2021
Merged

Removing gemini #822

merged 13 commits into from
Mar 12, 2021

Conversation

dannylamb
Copy link

JIRA Ticket: Goes with Islandora-Devops/isle-dc#132

What does this Pull Request do?

Uses the new EntityMapper service from Islandora/Crayfish-Commons#45 to recreate what GeminiLookup did. Deletes a lot of stuff 🔥

How should this be tested?

  • Pull in this PR to your Islandora module
  • Pull in No gemini Crayfish-Commons#45 to crayfish-commons in your sites vendor/islandora/crayfish-commons directory.
  • Clear the cache
  • Make a repository item and give it an Original File
  • Confirm the Fedora URL is correct for
    • The node
    • The Original File media (which lives in Fedora)
    • A derivative (which does not live in Fedora)
    • A taxonomy term (I looked at the Model of the node)

Interested parties

@Islandora/8-x-committers

@elizoller
Copy link
Member

@dannylamb is this ready for testing?

@dannylamb
Copy link
Author

@dannylamb
Copy link
Author

Can't seem to replicate using PHP 7.4 and Drupal 9.1.4

I'm noticing that we're on 9.1.1. Maybe i just need to get that to bump.

@dannylamb dannylamb marked this pull request as ready for review February 4, 2021 15:44
@elizoller
Copy link
Member

elizoller commented Feb 10, 2021

So i did the following

  1. spin up new playbook (fedora 5, php 7.4, drupal 9)
  2. go into islandora module and check out no-gemini branch
  3. go into crayfish and check out no-gemini branch
  4. find all vendor directories with crayfish-commons and go into those and check out no-gemini branch
  5. drush cr
  6. make items ( i did so with the migrate_islandora_csv module because i didn't want to do it manually through the UI)

I didn't get any derivs for the migrated objects.
I suspect I'm missing some config changes somewhere?

for example the milliner log contains

[2021-02-10 13:54:21] app.DEBUG: > POST /milliner/media/field_media_image [] []
[2021-02-10 13:54:21] app.ERROR:  {"Exception":"[object] (GuzzleHttp\\Exception\\ConnectException(code: 0): cURL error 6: Could not resolve host: default (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) at /var/www/html/Crayfish/Milliner/vendor/guzzlehttp/guzzle/src/Handler/CurlFactory.php:200)"} []
[2021-02-10 13:54:21] app.DEBUG: < 500 [] []

When I checked on a node page, i saw a fedora URI and i clicked it and it returned a 404, so i did index in fedora action and then clicked on the fedora URI again and it worked (no 404).

Media Fedora URIs appear to be working. but i didn't get any derivs for the migrated objects.

I will note that when i manually made an item node in the UI, it did persist to fedora without issue.
When i made a media manually through the UI, it had no issue with derivs.

islandora.module Outdated
$media_source_service = \Drupal::service('islandora.media_source_service');
$source_file = $media_source_service->getSourceFile($entity);
$uri = $source_file->getFileUri();
$scheme = \Drupal::service('file_system')->uriScheme($uri);
Copy link
Member

Choose a reason for hiding this comment

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

This is deprecated in D9:
Replaced with \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface::getScheme()

@dannylamb
Copy link
Author

I'll try and follow your steps on a playbook and see what's up with derivatives.

@dannylamb
Copy link
Author

Ok, after fixing that deprecation you found (which for me that class was straight up gone on 9.2.0-dev), I've got it working on a fresh playbook. No issues with derivatives that I can see.

Here's how I installed it (scraped from my bash history)

cd /var/www/html/drupal/web/modules/contrib/islandora
git fetch origin
git checkout no-gemini 
cd /var/www/html/drupal/vendor/islandora/crayfish-commons/
git fetch origin
git checkout no-gemini 
cd /var/www/html/Crayfish/
sudo git fetch origin
sudo git checkout no-gemini 
cd Milliner/vendor/islandora/crayfish-commons/
sudo git fetch origin
sudo git checkout no-gemini 
cd /var/www/html/drupal/
drush cr

Then I made an Image and uploaded an Original file and everything checked out. Everything makes its way into Fedora as expected without going through Gemini.

@dannylamb
Copy link
Author

Ah... and now re-reading... Maybe the issue is just with migrate_islandora_csv. I'll give that a whirl.

@dannylamb
Copy link
Author

@elizoller I bumped right into what you experienced. Turns out that I was calling the migration wrong. It needed both --userid=1 and --uri=localhost:8000 in order to work. After that it worked fine. By doing this I managed to test https://github.com/Islandora/migrate_islandora_csv/pull/40/files for ya. Works good!

Here's how I got and ran the code:

cd /var/www/html/drupal/web/modules/contrib
git clone https://github.com/asulibraries/migrate_islandora_csv.git
cd migrate_islandora_csv/
git checkout d9_updates
drush en migrate_islandora_csv
drush mim --userid=1 --uri=http://localhost:8000 --group migrate_islandora_csv

@elizoller
Copy link
Member

did that fix all the derivative problems? if so, i'll re run and see what i get.

@dannylamb
Copy link
Author

Derivatives worked out fine once I ran it with the right arguments.

@elizoller
Copy link
Member

ok, i'll have another look

@dannylamb
Copy link
Author

🙇

@elizoller
Copy link
Member

@dannylamb i pulled in the new commit, rollback the migrations, re-imported them with the uri and userid flags and confirmed files, media, and nodes were all created. all are visible in drupal side, all expected derivatives were created, and fedora URI resolves correctly.
what else can i test for this?

@dannylamb
Copy link
Author

dannylamb commented Mar 1, 2021

I think that's about it as far as module code. I have ISLE rigged up to use this. I think the playbook will be mostly unaffected other than we'd want to remove the stuff to deploy gemini. Would you be willing to test if I throw up a quick PR to islandora-playbook?

@elizoller
Copy link
Member

Definitely

@dannylamb
Copy link
Author

🙇‍♂️

@dannylamb
Copy link
Author

...and seeing now that all this stuff totally breaks the recast service 🎉

I don't think it's too bad since the service in Crayfish commons works both ways, but still. One more hurdle 🤕

@elizoller
Copy link
Member

🤕

@dannylamb
Copy link
Author

Ok, I've got recast working again. Fixing up its tests... all most there....

@dannylamb
Copy link
Author

I just pushed up commits to Islandora/Crayfish#111 if you want to test again.

@dannylamb
Copy link
Author

On to the playbook pull..

@elizoller
Copy link
Member

@dannylamb any interested in #828 as a potential substitute for the sed line in this PR?

@elizoller
Copy link
Member

@dannylamb any interest in #828 as a potential substitute for the sed line in this PR?

@dannylamb
Copy link
Author

sed is gone 🔥

old habits die hard, i guess. didn't even look to the line above, which is a perfectly good example of setting a property using the command line 😅

Copy link
Member

@elizoller elizoller left a comment

Choose a reason for hiding this comment

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

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