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.

Sign up for GitHub

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

Jump to bottom

Feature/685 update container config properties usage #751

Merged
merged 23 commits into from
Jun 11, 2024
Merged

Feature/685 update container config properties usage #751

merged 23 commits into from
Jun 11, 2024

Conversation

pnoltes
Copy link
Contributor

@pnoltes pnoltes commented Jun 5, 2024

This PR introduces the use of JSON properties encoding for Apache Celix containers.

It updates all the config.properties files to JSON format and modifies the handling of PROPERTIES in the CMake add_celix_container function to generate JSON.

Additionally:

  • It improves error handling, with the updated Celix properties functions now returning a celix_status_t.
  • It refactors the Celix launcher for better error handling and increased code coverage.
  • It removes and replaces the old properties load/store functions.

pnoltes added 17 commits May 24, 2024 12:54
@pnoltes
gh-685: Remove of old properties load and store
2c28348 
Also introduces some additional error test to check
the handling of properties load/save with an error
return.
@pnoltes
gh-685: Refactor config prop cmake gen and test config props
74f4257
@pnoltes
gh-685: Improve error handling for properties save/load
0a0709a
@pnoltes
gh-685: Rename properties load2/loadFromString2 to load/loadFromString
0566315
@pnoltes
gh-685: Change space sep in auto start bundle to comma seperator
b8c6ab5
@pnoltes
gh-685: Fix properties encoding usage in rsa shm
55b3ae8
@pnoltes
gh-685: Fix macos specific file utils tests
7405378
@pnoltes
gh-685: Fix macos specific file utils tests
68098de
@pnoltes
gh-685: Fix memleak in bundle archive
64e3647
@pnoltes
gh-685: Fix memleak in bundle archive
0c859f0
@pnoltes
gh-685: Refactor celix launcher
ddf0a77
@pnoltes
Merge branch 'refs/heads/master' into feature/685-update-container-co…
87ab2b3 
…nfig-properties-usage
@pnoltes
gh-685: Replace sigabbrev_np usage with strsignal
5b4cd3a
@pnoltes
gh-685: Replace constexpr with define
fe5aa59
@pnoltes
gh-685: Add celix launcher test suite with error injection
25df868
@pnoltes
gh-685: Fix tms_tests and improve several error handling issues
e06e2ad
@pnoltes
gh-685: Improve celix_launcher coverage and fix memleaks
bf0011a
@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 93.53535% with 32 lines in your changes missing coverage. Please review.

Project coverage is 90.24%. Comparing base (fd4b138) to head (f237f4b).
Report is 3 commits behind head on master.

Files Patch % Lines
libs/framework/src/framework.c 85.47% 26 Missing ⚠️
libs/framework/src/celix_launcher.c 97.76% 3 Missing ⚠️
..._service_admin_shm_v2/rsa_shm/src/rsa_shm_server.c 66.66% 1 Missing ⚠️
libs/framework/src/bundle_archive.c 95.65% 1 Missing ⚠️
libs/framework/src/celix_bundle_cache.c 95.23% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #751      +/-   ##
==========================================
+ Coverage   90.20%   90.24%   +0.04%     
==========================================
  Files         225      226       +1     
  Lines       26393    26281     -112     
==========================================
- Hits        23807    23718      -89     
+ Misses       2586     2563      -23

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PengZheng PengZheng self-requested a review June 6, 2024 01:40
PengZheng
PengZheng requested changes Jun 6, 2024
View reviewed changes
Copy link
Contributor

@PengZheng PengZheng left a comment

Choose a reason for hiding this comment

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

Both the API cleanup and move of config.properties to JSON format are nice. I have some remarks to make.

libs/framework/src/celix_bundle_cache.c Outdated Show resolved Hide resolved
libs/framework/src/framework.c Show resolved Hide resolved
libs/framework/src/framework.c Outdated Show resolved Hide resolved
libs/framework/src/framework.c Show resolved Hide resolved
libs/framework/src/framework.c Outdated Show resolved Hide resolved
libs/framework/src/celix_launcher.c Outdated Show resolved Hide resolved
libs/framework/src/celix_launcher_private.h Outdated Show resolved Hide resolved
libs/framework/src/framework.c Outdated Show resolved Hide resolved
bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c Show resolved Hide resolved
cmake/cmake_celix/ContainerPackaging.cmake Outdated Show resolved Hide resolved
PengZheng and others added 3 commits June 7, 2024 12:20
@PengZheng
Make various lock guard deinit explicitly callable so that locks can …
a014956 
…be released manually.
@pnoltes
gh-685: Add bundle entry guard
8b0e0e0 
Also improves several small issues based on
review comments.
@pnoltes
gh-685: Add rsa shm test case
f237f4b 
And some small improvements based on review
comments
@pnoltes pnoltes requested a review from PengZheng June 9, 2024 16:52
PengZheng
PengZheng reviewed Jun 10, 2024
View reviewed changes
Copy link
Contributor

@PengZheng PengZheng left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please note that there are remarks for launcher in hidden conversations. And I have addressed the remaining ones in framework.c.

libs/framework/src/framework.c Outdated Show resolved Hide resolved
libs/framework/src/framework.c Outdated Show resolved Hide resolved
@pnoltes
Copy link
Contributor Author

pnoltes commented Jun 10, 2024

Please note that there are remarks for launcher in hidden conversations. And I have addressed the remaining ones in framework.c.

I forgot the launcher and global framework remarks, should be updated now.

@pnoltes pnoltes requested a review from PengZheng June 10, 2024 18:19
@PengZheng
gh-685: Improve error handling of launcher.
621c2be 
1. Avoid discarding valid configuration on properties creation error.
2. Fix leak on cache creation error.
PengZheng
PengZheng approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@PengZheng PengZheng left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

@pnoltes pnoltes merged commit 58860bb into master Jun 11, 2024
32 checks passed
@pnoltes pnoltes deleted the feature/685-update-container-config-properties-usage branch June 11, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PengZheng PengZheng PengZheng approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pnoltes @codecov-commenter @PengZheng