machine icon indicating copy to clipboard operation
machine copied to clipboard

Error parsing Paratext settings

Open Enkidu93 opened this issue 1 year ago • 4 comments

We got this error recently on ext QA:

An unhandled exception was thrown by the application.
System.ArgumentException: An item with the same key has already been added. Key: SIL.Scripture.Versification+Table+VersificationKey
at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
at SIL.Scripture.Versification.Table.Load(TextReader stream, String fullPath, ScrVers baseVers, String name)
at SIL.Machine.Corpora.ParatextProjectSettingsParserBase.Parse()

Enkidu93 avatar Sep 10 '24 21:09 Enkidu93

This is actually a bug (or not proper handling) in SIL. Scruipture, that is, https://github.com/sillsdev/libpalaso -> https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Versification.cs.

johnml1135 avatar Sep 11 '24 11:09 johnml1135

OK, is this something we should try to fix there or something we need to handle more robustly in Machine itself?

Enkidu93 avatar Sep 12 '24 13:09 Enkidu93

@ddaspit - what do you think? Should we make a pull request to https://github.com/sillsdev/libpalaso?

johnml1135 avatar Sep 12 '24 13:09 johnml1135

Yes, we need to make a change to SIL.Scripture. Versification.Table.Implementation is a static property, and the Load method saves custom versifications to an internal dictionary. This will result in the internal dictionary filling up with custom versifications over time. We don't want or need that. We want a Load method that does not add custom versifications to the dictionary.

ddaspit avatar Sep 12 '24 14:09 ddaspit

We've seen a few more of these errors on the rancher alerts, so maybe we should escalate this issue. It also shouldn't be terribly difficult to fix, right? And there might be a slow turnaround given that we might need to wait for approval from outside our team?

Enkidu93 avatar Dec 02 '24 16:12 Enkidu93

@Enkidu93 - can you make a issue (and propose a fix) on libpalaso? They are usually pretty responsive, but it still will likely take 2 weeks start to finish. Feel free to message the repo owners on slack as well.

johnml1135 avatar Dec 03 '24 12:12 johnml1135

@Enkidu93 - what is the current status of this? Do we need to use https://www.nuget.org/packages/ParatextData?

johnml1135 avatar Jan 15 '25 16:01 johnml1135

@Enkidu93 - what is the current status of this? Do we need to use https://www.nuget.org/packages/ParatextData?

Yes, we can either use ParatextData or implement it ourself following ParatextData's implementation.

Enkidu93 avatar Jan 15 '25 17:01 Enkidu93

I don't want to have a dependency on ParatextData. We should be able to implement it ourselves pretty easily.

ddaspit avatar Jan 15 '25 17:01 ddaspit

I'm going to try to put a PR into SIL.Scripture today. Not sure how long that review process will be.

Enkidu93 avatar Feb 12 '25 15:02 Enkidu93