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

Error parsing Paratext settings #241

Open
Enkidu93 opened this issue Sep 10, 2024 · 9 comments · May be fixed by #283
Open

Error parsing Paratext settings #241

Enkidu93 opened this issue Sep 10, 2024 · 9 comments · May be fixed by #283
Assignees

Comments

@Enkidu93
Copy link
Collaborator

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 Enkidu93 added this to Serval Sep 10, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in Serval Sep 10, 2024
@johnml1135
Copy link
Collaborator

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.

@Enkidu93
Copy link
Collaborator Author

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

@johnml1135
Copy link
Collaborator

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

@ddaspit
Copy link
Contributor

ddaspit commented Sep 12, 2024

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.

@Enkidu93
Copy link
Collaborator Author

Enkidu93 commented Dec 2, 2024

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?

@johnml1135
Copy link
Collaborator

@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
Copy link
Collaborator

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

@Enkidu93
Copy link
Collaborator Author

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

@ddaspit
Copy link
Contributor

ddaspit commented Jan 15, 2025

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

@Enkidu93 Enkidu93 linked a pull request Jan 21, 2025 that will close this issue
@Enkidu93 Enkidu93 moved this from 🆕 New to 👀 In review in Serval Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging a pull request may close this issue.

3 participants