ADDR-132: Add 'Get Global Properties' proxy privilege #41
Conversation
|
Can you look into the above failure on Java 17? |
|
Hi @dkayiwa, I am done! |
|
It has failed again. |
Seremba
force-pushed
the
ADDR-132
branch
2 times, most recently
from
e4a43fd to
6ebfe8e
Compare
|
Hello @dkayiwa , how about now? |
|
Hi @dkayiwa I am done making changes to this. Kindly review! |
|
Hi @dkayiwa |
|
Hi @dkayiwa, I set this to draft until the tests pass. However, currently, OpenMRS does not support Java 17. Don't you think it is the cause for failures until maybe OpenMRS officially supports Java 17? What do you have to say? |
You are correct! |
|
Thanks for the response @dkayiwa. What do you suggest I should do? |
Let us start by removing all the changes we had added for Java 17 |
Hi @dkayiwa, I am done already |
|
I still see more changes than i would expect. |
|
I am done @dkayiwa |
|
Your pull request still has very many changes not related to the Get Global Properties privilege. |
|
Hi @dkayiwa, I am done. |
| // only initialize if necessary (and if we have entries) | ||
| if ((this.fullAddressCacheInitialized == false || MapUtils.isEmpty(this.fullAddressCache)) | ||
| && this.getAddressHierarchyEntryCount() > 0) { | ||
|
|
||
| this.fullAddressCache = new HashMap<Locale, Map<String,List<String>> >(); |
There was a problem hiding this comment.
Can we revert the formatting changes?
|
Hi @dkayiwa I am done reverting the formatting changes |
|
Do we need to remove the proxy privilege? |
|
You mean with the try-finally block? I got an error with that and the problem did not get away. But let me try again |
Hi @dkayiwa, I added and removed the proxy privilege with a try-finally block; compiled and started the reference application, and reloaded the address hierarchy module. Stopped the server, and ran the reference application again. Then left the program running throughout the night so I confirm whether |
|
Hi @dkayiwa , is there anything you require I do on this? |
| getAddressesForLocale(locale); | ||
| this.fullAddressCacheInitialized = true; | ||
| try { | ||
| Context.addProxyPrivilege("Get Global Properties"); |
There was a problem hiding this comment.
I would use a locally declared constant for the above string.
There was a problem hiding this comment.
I would use a locally declared constant for the above string.
Hi @dkayiwa, I am done with this!
|
Are you now able to run this module without errors in your modified openmrs-core? |
Yes. I can run the address hierarchy module without any errors. |
|
Is it the only module that you have loaded? Or do you even have the rest of the reference application modules? |
|
You mean loading and unloading the rest of the reference application modules? |
|
I mean being able to run all the reference application modules with your changes in openmrs-core |
They also run well. I get no problem whatsoever with any reference application module. |
|
How many reference application modules do you have loaded? |
|
I have about 42 modules in the reference application cc @dkayiwa |
|
So if those are successfully running and you have done all the required changes in openmrs-core as specified in that ticket, then you can ask for code review as advised here: https://openmrs.atlassian.net/wiki/spaces/docs/pages/25477199/Pull+Request+Tips |
|
Thank you so much! Let me do that. |
Description of what I changed
Issue I worked on
see https://openmrs.atlassian.net/browse/ADDR-132
Checklist: I completed these to help reviewers :)
My IDE is configured to follow the code style of this project.
No? Unsure? -> configure your IDE, format the code and add the changes with
git add . && git commit --amendI have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)
No? -> write tests and add them to this commit
git add . && git commit --amendI ran
mvn clean packageright before creating this pull request andadded all formatting changes to my commit.
No? -> execute above command
All new and existing tests passed.
No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.
My pull request is based on the latest changes of the master branch.
No? Unsure? -> execute command
git pull --rebase upstream master