add routeIds and descriptions #47
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@acoburn testing now 😄 |
|
@acoburn @daniel-dgi not seeing any sync action to fcrepo from Drupal 😢 |
|
somehwere in that wall of text is what's up with the xml. you can ignore all the scripting factory nonsense. if you install camel-script wihotut camel-script-jruby or camel-script-groovy, you get those errors. |
|
... what @daniel-dgi said. But actually, I'd hold off on this one and #48 for now. I think having a testing framework in place first makes more sense. And once #46 is merged, we can move forward with that. |
|
at a closer look, no route with id 'get'. so something's expecting it and it's not there, or there's a naming conflict on 'get' |
|
@daniel-dgi is correct -- I renamed some of the context ids (there were several "get" ids, etc), but that is causing other errors. In short, this PR has some problems. |
|
@daniel-dgi, newbie here: maybe we need to update those https://github.com/Islandora-Labs/islandora/blob/b7beb393a1713f2b131be95d5878a7e05eed88b7/camel/services/basic-image-service/src/main/resources/OSGI-INF/blueprint/blueprint.xml#L20-L23 |
|
@DiegoPino makes sense to me. |
|
You nailed it. getBasicImage, postBasicImage, etc... |
Update for #47, also changed the individual route definition file names just to be consistent on current naming.
|
@ruebot, i think we are ready for another test. 🙏 |
|
@DiegoPino++ -- pulling down latest updates, and doing a vagrant up now 😄 |
|
We can create collection objects and basic image objects, but the image derivatives are not created: |
update to previous #47, forgot to update routeContextRef for sync service
|
Testing again! |
|
@DiegoPino++ @acoburn++ perfecto! Now, we just need to merge with 7.x-2.x branch since we merged #46, and we should be good to go. More than happy to test again once that is in 😄 |
|
@ruebot++ , will give this a try tomorrow. Thanks a lot for testing |
Resolving conflicts with 7.x-2.x - camel_addRouteIds
|
I've rebased this against master, so it should be easier to test |
|
@DiegoPino I clearly screwed that up. I will merge https://github.com/acoburn/islandora/pull/2 and https://github.com/acoburn/islandora/pull/3 back into this PR. |
|
hey @acoburn, don't worry, i will do this later. |
…ndora into camel_addRouteIds
|
@DiegoPino your commits are now part of this PR |
|
@acoburn++, you are awesome, thanks a lot! |
|
Guess who is testing again!? |
|
@DiegoPino @acoburn double check the rebase again? The image derivative service isn't working now 😢 ...I'm not seeing the binary object attach to the basic image object either. |
|
@ruebot or @acoburn, don't want to be a pain, but did you test on a fresh vagrant (destroy - up) after #46? I was searching for the problem on this one so i did a pre-test using current 7.x-2.x only. I got a non functional islandora2, karaf did not deploy the features. I get warnings during maven compile (no fatal errors so the vagrant spins, but i think there are some references wrong) , my humble opinion is that #46 is not complete. E.g, the karaf deployment scripts don't match the new unified structure. Try /opt/karaf/bin/client < $HOME/islandora/install/configs/karaf/services.script, you will get errors. |
|
@DiegoPino I did a fresh |
|
@ruebot @DiegoPino my approach is somewhat different, which is to start writing a test suite. There are things in this PR that I need for the tests, so I'm wondering whether to a) add stubs for now (i.e. route IDs) and handle any conflicts later or b) hold off on this PR until the tests are farther along or c) cherry pick from this PR and integrate those changes into the test suite as I go along. I'd like to do the tests in stages. Once I get a framework in place, you'll easily see how to contribute to the test -- testing in camel is not as straight forward as simply writing the routes! |
|
@acoburn My vote would be for option a. We have some stub test suite tickets open (#41 & #42) that we could address after we get this in. I'm also watching the build now, and looking for @DiegoPino's suspicion that we might need to update the build scripts, and that I might have missed something with vagrant in #46. ...but! Moving forward, I agree that we should aim for more test driven development 😄 |
|
hrm... Everything looked fine on this build. This time everything seems fine. When I add a basic image, image is attached, and derivative is created. Only difference this time around, I'm on my desktop at home instead of my work desktop. |
|
@ruebot - did a second clean vagrant without luck. I'm clueless? |
|
doing a sanity build on 7.x-2.x |
|
7.x-2.x sane. rebuilding this branch again |
|
Good on a second sanity here at e3298f5 👍 |
|
closing this for now -- I may reopen it later, but I'd like to see a testing framework in place first, similar to #65 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's good practice to have distinctive routeId values (and descriptions) for your camel routes, especially when using JMX or other monitoring services (such as hawt.io).