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

Fatal error because of using Log4J 1.X dependencies without adding them as direct dependency #334

Closed
mhm90 opened this issue Aug 15, 2022 · 7 comments

Comments

@mhm90
Copy link

mhm90 commented Aug 15, 2022

Hi,

My project uses Log4j 2 as logging implementation and this causes some Log4j 1.X dependencies to be overridden by Log4j2. But some classes in the psl-core module use these old dependencies. You should consider them as a direct dependency if you are using them directly in your code. Here is what I found on maven built jar: 'org.linqs:psl-core:CANARY-2.3.1' (not the current version of repo!):
class: org.linqs.psl.config.Config:

import org.apache.log4j.helpers.Loader;
import org.apache.log4j.helpers.OptionConverter;

I noticed that these usages are removed from the repository. Could you consider merging a hotfix?

@eriq-augustine
Copy link
Member

Thanks for the find.

Would it be viable for you to try the recent 2.3.0 release?
A hotfix could be possible, but it would be for 2.2.2, since we typically don't want to hotfix canary builds.

@mhm90
Copy link
Author

mhm90 commented Aug 16, 2022

Actually, I waited so long for PSL-VMI to be merged. Is there any plan for the next release?

@mhm90
Copy link
Author

mhm90 commented Aug 16, 2022

since we typically don't want to hotfix canary builds

I already know this is fixed now in your code, but unfortunately this CANARY-2.3.1 build has also a fatal bug in org.linqs.psl.database.rdbms.DataStoreMetadata where you are using value as the column name which is a reserved word in SQL language:

    private void initialize() {
        if (exists()) {
            return;
        }

        List<String> sql = new ArrayList<String>();
        sql.add("CREATE TABLE IF NOT EXISTS " + METADATA_TABLENAME + " (");
        sql.add("  namespace VARCHAR(255),");
        sql.add("  keytype VARCHAR(255),");
        sql.add("  keyvalue VARCHAR(255),");
        sql.add("  value VARCHAR(255),");   // <-- HERE
        sql.add("  PRIMARY KEY(namespace, keytype, keyvalue)");
        sql.add(")");

        try (
            Connection connection = dataStore.getConnection();
            PreparedStatement statement = connection.prepareStatement(ListUtils.join(System.lineSeparator(), sql));
        ) {
            statement.execute();
        } catch (SQLException ex) {
            throw new RuntimeException("Error creating metadata table.", ex);
        }
    }

So, I think you should reconsider applying hotfixes on CANARY builds!

@eriq-augustine
Copy link
Member

eriq-augustine commented Aug 16, 2022

Unfortunately, the original developers of PSL-VMI have moved on to other projects, so we have no plans to merge it in.

I'm still curious if you have tried/considered version 2.3.0.
Since canary builds are release candidates, version 2.3.0 is essentially the patched version of any CANARY-2.3.x build.

If you can't use 2.3.0, then you may have to just build locally.

@mhm90
Copy link
Author

mhm90 commented Aug 16, 2022

Since canary builds are release candidates, version 2.3.0 is essentially the patched version of any CANARY-2.3.x build.

Oh, dear! I was thinking any 2.3.1 is after any 2.3.0. Do you mean 2.3.0 is a later version than CANARY-2.3.1 and the above issues are fixed in that?

Unfortunately, the original developers of PSL-VMI have moved on to other projects, so we have no plans to merge it in.

Actually, I am planning to fork the repo and avoid the generation of the JSON file but add a callback for the user in order to send it to something like Graylog as an automated feature in a backend or just serialize it by file as a simple PSL run. It is not the best place for discussing it, but could you give it a try and help me with that in order to release it as a feature in the main repo?

Thanks

@eriq-augustine
Copy link
Member

Oh, dear! I was thinking any 2.3.1 is after any 2.3.0. Do you mean 2.3.0 is a later version than CANARY-2.3.1 and the above issues are fixed in that?

Yeah, we treat canaries similar to release candidates where the major/minor number indicate the release it is a candidate for and the patch number indicates the number of the release candidate.
https://psl.linqs.org/wiki/master/Working-With-Canary.html

2.3.0 is the most resent release and was put out about a month ago.

It is not the best place for discussing it, but could you give it a try and help me with that in order to release it as a feature in the main repo?

I can provide feedback, but probably won't provide any implementation for a while.
However, with our new runtime and proposed new JSON config (#274), we are going in a direction that may encapsulate all the PSL-VMI data exporting.
Feel free to take that to another issues and let us know all the data you want exported.

@mhm90
Copy link
Author

mhm90 commented Aug 16, 2022

2.3.0 is the most resent release and was put out about a month ago.

Ok then, that will fix my issue.

However, with our new runtime and proposed new JSON config (#274), we are going in a direction that may encapsulate all the PSL-VMI data exporting.

I think I misled you by the word JSON. It is better to close this issue and discuss it in another one, since my problem is fixed by 2.3.0 version.

@mhm90 mhm90 closed this as completed Aug 16, 2022
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