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

eddn: Address EDDN server complains about some codexentry messages #1426

Merged
merged 8 commits into from
Feb 3, 2022

Conversation

Athanasius
Copy link
Contributor

I still can't explicitly explain what causes these errors. All my example events have all the correct data. So this turned into tracing the code and adding some more paranoid checks and logging.

Closing #1393

@Athanasius Athanasius added the EDDN label Feb 1, 2022
@Athanasius Athanasius added this to the 5.3.0 milestone Feb 1, 2022
@Athanasius Athanasius self-assigned this Feb 1, 2022
As it was we'd blindly set codexentry['BodyName'] from it if not None.
@Athanasius Athanasius marked this pull request as ready for review February 1, 2022 17:26
plugins/eddn.py Outdated Show resolved Hide resolved
plugins/eddn.py Outdated Show resolved Hide resolved
plugins/eddn.py Outdated Show resolved Hide resolved
@Athanasius Athanasius changed the title eddn: Fix EDDN server complaints about codexentry messages eddn: Address EDDN server complains about some codexentry messages Feb 2, 2022
@Athanasius
Copy link
Contributor Author

I'm still pondering tweaking this some to not send any message that's found to have empty strings just before queueing it up.

Ideally this can then set status text to alert the user: Bad CodexEntry - please alert developer because I really want to know which Journal events are tripping this up.

@A-UNDERSCORE-D
Copy link
Contributor

That works for me

@Athanasius Athanasius merged commit 918272b into develop Feb 3, 2022
@Athanasius Athanasius deleted the fix/1393/eddn-codexentry_empty-string branch February 3, 2022 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants