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

Switch Homarus to Symfony 4 #102

Closed
wants to merge 9 commits into from
Closed

Switch Homarus to Symfony 4 #102

wants to merge 9 commits into from

Conversation

whikloj
Copy link
Member

@whikloj whikloj commented Apr 8, 2020

GitHub Issue: Islandora/documentation#1224

** DO NOT MERGE FIRST **

The sequence for the related PRs is

What does this Pull Request do?

Switches Homarus from Silex to Symfony 4.
Updates Houdini to symfony 4.4

How should this be tested?

It should work the same as before, no difference when uploading images, audio or video files.

Interested parties

@Islandora/8-x-committers

@codecov
Copy link

codecov bot commented Apr 8, 2020

Codecov Report

Merging #102 (38a0611) into dev (291ebe0) will decrease coverage by 2.18%.
The diff coverage is 67.76%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #102      +/-   ##
============================================
- Coverage     91.87%   89.68%   -2.19%     
- Complexity      175      182       +7     
============================================
  Files             9        9              
  Lines           726      708      -18     
============================================
- Hits            667      635      -32     
- Misses           59       73      +14     
Impacted Files Coverage Δ Complexity Δ
Gemini/src/Controller/GeminiController.php 98.24% <ø> (ø) 19.00 <0.00> (ø)
Recast/src/Controller/RecastController.php 67.54% <35.29%> (-6.80%) 35.00 <2.00> (-1.00)
Milliner/src/Service/MillinerService.php 88.39% <39.02%> (-1.46%) 55.00 <6.00> (-1.00)
Milliner/src/Controller/MillinerController.php 92.22% <86.95%> (-4.81%) 24.00 <8.00> (ø)
Gemini/src/UrlMapper/UrlMapper.php 100.00% <100.00%> (ø) 17.00 <2.00> (+9.00)
Homarus/src/Controller/HomarusController.php 96.61% <100.00%> (-3.39%) 13.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44eaba6...38a0611. Read the comment docs.

@elizoller
Copy link
Member

Islandora/Crayfish-Commons#39 has been merged

@elizoller
Copy link
Member

@whikloj is this one next? or is there a step between?

@whikloj
Copy link
Member Author

whikloj commented Dec 29, 2020

This one is next, but I need to update the composer.json to point back at dev-symfony-flex and clean up the merge conflicts. I'll try to do that this week, if not then definitely when I'm back on Tuesday.

@elizoller
Copy link
Member

great, thanks!

@whikloj whikloj force-pushed the ISLANDORA-1224-homarus branch from bd9e01f to 88bc94e Compare December 30, 2020 18:06
@whikloj
Copy link
Member Author

whikloj commented Dec 31, 2020

I'm done here, there is some methods with no test coverage so this is not going to pass CodeCov. Specifically the MillinerService method getFileFromMedia is not covered. I didn't want to try as I'm out of touch on its function.

@elizoller
Copy link
Member

@whikloj would the best method for testing be to checkout this branch, do a composer install, and make sure derivatives are still properly made?

@whikloj
Copy link
Member Author

whikloj commented Jan 4, 2021

@elizoller you can use this branch Islandora-Devops/islandora-playbook#175 to pull in the new Crayfish.

@whikloj
Copy link
Member Author

whikloj commented Jan 4, 2021

If you want to pull the branch in you can, but you will need to do the configuring which you can see at this PR (islandora-deprecated/ansible-role-crayfish#34) I think

@elizoller
Copy link
Member

I attempted to spin up a box with the islandora-playbook#175 PR, that didn't work due to composer dependency issues. So I just manually made the changes in that PR over a fresh dev islandora-playbook (so it would use the whikloj branch). Once I had the box up, I verified that Crayfish was checked out to the correct branch. Then I tried to create an object in islandora, but I'm getting 500s from Gemini

[Wed Jan 06 16:09:50.423060 2021] [php7:error] [pid 25493] [client 127.0.0.1:47322] PHP Fatal error:  Class Islandora\\Crayfish\\Commons\\Syn\\JwtAuthenticator contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Symfony\\Component\\Security\\Guard\\AuthenticatorInterface::supports) in /var/www/html/Crayfish/Gemini/vendor/islandora/crayfish-commons/src/Syn/JwtAuthenticator.php on line 16

@whikloj
Copy link
Member Author

whikloj commented Jan 6, 2021

Clearly updating all the composer files for composer 2 was a bad idea. This might be a good time to trash the entire PR and have someone start over.

@elizoller
Copy link
Member

really? i'd hate to lose all this work!
maybe it would help if danny's PR to remove gemini goes through and then this could rebased over that so it removes at least one component?

@whikloj
Copy link
Member Author

whikloj commented Jan 6, 2021

The problem is that with all the microservices tied together. This PR never touched Gemini until recently to make Travis pass. So I don't know what is going on with that.

@whikloj
Copy link
Member Author

whikloj commented Nov 22, 2021

Closed in favour of #133

@whikloj whikloj closed this Nov 22, 2021
@whikloj whikloj deleted the ISLANDORA-1224-homarus branch November 22, 2021 16:27
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