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

Remove joda time dependency from netflix-sel module #61

Open
jun-he opened this issue Jul 26, 2024 · 5 comments
Open

Remove joda time dependency from netflix-sel module #61

jun-he opened this issue Jul 26, 2024 · 5 comments
Assignees

Comments

@jun-he
Copy link
Contributor

jun-he commented Jul 26, 2024

To remove it, we have to implement those SEL methods using Java time libraries than Joda time.

@pranaybattu
Copy link

@jun-he Could you assign this issue to me. I would like to work on it.

@pranaybattu
Copy link

@jun-he

I’m starting on the task to remove the Joda-Time dependency from the Netflix-SEL module and replace it with Java's java.time API. Here’s my plan for the migration:

Migration Plan

1. Affected Parts in netflix-sel:

  • com.netflix.sel.type Package:

    • SelJodaDateTimeDays
    • SelJodaDateTimeFormatter
    • SelJodaDateTimeProperty
    • SelJodaDateTime
    • SelJodaDateTimeZone
  • com.netflix.sel.security Package:

    • SelClassLoader
  • Test Module:

    • SelJodaDateTimeDaysTest
    • SelJodaDateTimeFormatterTest
    • SelJodaDateTimePropertyTest
    • SelJodaDateTimeTest
    • SelJodaDateTimeZoneTest
    • MockTypeTest

2. Update Documentation:

  • Update README.md and any other documentation files to reflect the migration from Joda-Time to java.time.

3. Update Main Classes:

  • Replace Joda-Time classes and methods with equivalent java.time classes and methods.
  • Ensure all date-time manipulations and formatting are correctly migrated.

4. Update Test Classes:

  • Refactor test classes to use java.time.
  • Ensure all tests cover the same functionality as before.

5. Test and Validate:

  • Run all existing tests to ensure they pass.
  • Validate the correctness of the migration by verifying outputs and behaviors.

@pranaybattu
Copy link

Any input or concerns before I proceed? Let me know if there are specific things I should keep in mind!

Also, should we divide this into smaller tasks/issues, or handle it as a single issue?

@jun-he
Copy link
Contributor Author

jun-he commented Aug 2, 2024

@pranaybattu thanks for the interest and taking time to draft a plan. I assign this task to you.
To upgrade SEL, there are some investigations needed

  1. SEL is highly secured and restricted. It cannot even load a unloaded class into the class loader after initialized. So not sure if java date did any (e.g. start a thread or ask Java security manager for anything).
  2. Due to above issue, SEL have to cache all the loaded objects to avoid GC to unload them. Not sure how big the java time package is, comparing to jodatime.
  3. the jodatime interface is actually great. It's better to keep them. It means users will still use the same SEL functions to build their expression. We only rewrite the internal implementation by using java time.

@pranaybattu
Copy link

Sure, will explore those.

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

No branches or pull requests

2 participants