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

[Task]: Cleanup XML of transform WriteToLog #4145

Closed
nadment opened this issue Jul 20, 2024 · 6 comments · Fixed by #4146
Closed

[Task]: Cleanup XML of transform WriteToLog #4145

nadment opened this issue Jul 20, 2024 · 6 comments · Fixed by #4146
Assignees
Milestone

Comments

@nadment
Copy link
Contributor

nadment commented Jul 20, 2024

What needs to happen?

Cleanup XML of transform WriteToLog

Issue Priority

Priority: 3

Issue Component

Component: Metadata, Component: Transforms

@nadment nadment added this to the 2.10 milestone Jul 20, 2024
@nadment nadment self-assigned this Jul 20, 2024
@github-actions github-actions bot added P3 Nice to have Metadata labels Jul 20, 2024
@nadment nadment linked a pull request Jul 20, 2024 that will close this issue
@nadment
Copy link
Contributor Author

nadment commented Jul 21, 2024

Duplicate #1949

@nadment nadment changed the title [Task]: Cleanup XML of transform WriteToLog [Task]: Cleanup XML of transform WriteToLog Jul 25, 2024
hansva added a commit that referenced this issue Jul 31, 2024
Cleanup XML of transform WriteToLog #4145
@hansva
Copy link
Contributor

hansva commented Aug 1, 2024

Something has gone wrong with this

java.lang.NullPointerException: Cannot invoke "org.apache.hop.core.logging.LogLevel.ordinal()" because "loglevel" is null
	at org.apache.hop.pipeline.transforms.writetolog.WriteToLog.setLog(WriteToLog.java:130)
	at org.apache.hop.pipeline.transforms.writetolog.WriteToLog.processRow(WriteToLog.java:116)
	at org.apache.hop.pipeline.transform.RunThread.run(RunThread.java:54)
	at java.base/java.lang.Thread.run(Thread.java:840)

@hansva hansva reopened this Aug 1, 2024
@nadment
Copy link
Contributor Author

nadment commented Aug 1, 2024

Sorry, I trusted the Junit tests blindly !

The transform Log level property in XML:

  • before change: <loglevel>log_level_basic</loglevel>
  • after change: <loglevel>Basic</loglevel>

The WriteToLog action also uses <loglevel>Basic</loglevel>

I think we should keep the current form of storage.

We can renforce code, if it's null we use Basic as default.
We can also add a patch to deserialize Hop's value from 1.0 to 2.9 or agree to reset it to Basic by default ?

@hansva
Copy link
Contributor

hansva commented Aug 1, 2024

Yes I did I think something like that in the past accept all of the values but only save the correct ones

@nadment
Copy link
Contributor Author

nadment commented Aug 1, 2024

I have a fix for the NPE but for the moment it's not backwards compatible and I haven't found a way in LogLevel or XmlMetadataUtil to use an alias.

public static LogLevel lookupCode(final String code) {

// Try alias for WriteToLog transform compatibility version before 2.10
LogLevel alias = IEnumHasCode.lookupCode(LogLevel.class, "log_level_" + code, null);
if (alias != null) {
  return alias;
}

return IEnumHasCode.lookupCode(LogLevel.class, code, LogLevel.BASIC);

}

nadment added a commit to nadment/hop that referenced this issue Aug 1, 2024
nadment added a commit to nadment/hop that referenced this issue Aug 2, 2024
@nadment
Copy link
Contributor Author

nadment commented Aug 2, 2024

I can't find a way except to add a method to the IEnumHasCode interface and use it in XmlMetadataUtil.

public interface IEnumHasCode {
  String getCode();
  /** Returns a list of code aliases, useful for backward compatibility. */
  default List<String> getAlias() {
    return List.of();
  }
}

Someone might have a better idea?

hansva added a commit that referenced this issue Aug 5, 2024
Fix transform WriteToLog NPE #4145
@hansva hansva modified the milestones: 2.10, 2.11 Sep 25, 2024
@hansva hansva closed this as completed Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants