maestro icon indicating copy to clipboard operation
maestro copied to clipboard

Remove joda time dependency from netflix-sel module

Open jun-he opened this issue 1 year ago • 5 comments

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

jun-he avatar Jul 26 '24 23:07 jun-he

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

pranaybattu avatar Jul 27 '24 05:07 pranaybattu

@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 avatar Aug 01 '24 15:08 pranaybattu

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?

pranaybattu avatar Aug 01 '24 15:08 pranaybattu

@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.

jun-he avatar Aug 02 '24 13:08 jun-he

Sure, will explore those.

pranaybattu avatar Aug 02 '24 14:08 pranaybattu