Netflix / maestro

Maestro: Netflix’s Workflow Orchestrator
Apache License 2.0
3.32k stars 201 forks source link

Remove joda time dependency from netflix-sel module #61

Open jun-he opened 3 months ago

jun-he commented 3 months ago

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

pranaybattu commented 3 months ago

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

pranaybattu commented 3 months ago

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

2. Update Documentation:

3. Update Main Classes:

4. Update Test Classes:

5. Test and Validate:

pranaybattu commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

Sure, will explore those.