linqs / psl

The PSL software from the University of Maryland and the University of California Santa Cruz
http://psl.linqs.org
Apache License 2.0
296 stars 100 forks source link

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

Closed mhm90 closed 2 years ago

mhm90 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

mhm90 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.