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

Improve logging #5299

Merged
merged 11 commits into from
Sep 29, 2021
Merged

Improve logging #5299

merged 11 commits into from
Sep 29, 2021

Conversation

Articdive
Copy link
Member

@Articdive Articdive commented Sep 20, 2021

Description:

This pull request should improve the output of Towny in the case of it erroring out, it should also cleanup the startup process and make it easier to understand by moving most of it out of TownyUniverse and into Towny.

With this PR TownyUniverse only contains the database loading code and it's internal API. It no longer contains any config loading or similar that is all located in the main Towny class now. It also means there is less jumping from Towny <--> TownyUniverse if you follow onEnable(), which a lot of newer developers probably do.

IMPORTANT: This PR removes the old towny database outpost fixer that I wrote when I started working on Towny, it means that people can not directly update from < 0.91.4.14 to >= 0.97.2.0 (version including this PR). Pretty sure nobody runs that version.

Also means that outpostschecked.txt is no longer useful for anything.


New Nodes/Commands/ConfigOptions:

Nope


Relevant Towny Issue ticket:

Nope.


  • I have tested this pull request for defects on a server.

I have tested this PR but it is difficult to reproduce errors on purpose, I can't for example boot Towny in a read only environment to test such a case. Hence I can guarantee that Towny boots and acts normally exactly like before but we will have to wait for actual errors to know if this works.

By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the Towny License for perpetuity.

@Warriorrrr Warriorrrr added this to the 0.97.2.0 milestone Sep 20, 2021
Copy link
Member

@LlmDl LlmDl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to need to this to have more testing done on it, safemode is not longer thrown.

Good:

  • Errors on townyperms.yml failure to load point to line and column and can explain that tabs should not be used.
  • Errors on config town_level failure to load point to the line and column after the issue. Useful for someone that knows how to read a yaml error.
  • Errors on config failure to load point to the line and column which is missing the trailing '.

Bad:

  • Safemode is never thrown so an error on failure to load townyperms.yml results in Towny not protecting anything.

    • I added a tab to the first line below nomad:
    • Towny did not enable at all, making greifing possible.
  • Safemode is not thrown when the database.yml fails to load.

    • I changed the database save and load to squirrel.
    • Towny did not enable at all, making greifing possible.
  • Safemode is not thrown when the town_level is not valid yaml.

    • I removed a trailing ' from the namePostfix.
    • Resulted in a complete wipe of the config, Towny enabled. Versions without this logger improvement do not save a default config.yml when this happens.
  • Safemode is not thrown when the config is not valid yaml.

    • I removed a trailing ' from town.default_public ex: default_public: 'true
    • Resulted in a complete wipe of the config, Towny enabled. Versions without this logger improvement do not save a default config.yml when this happens.
  • Breaking the yaml of the global.yml file does show a large yaml error, but otherwise has no effect (other than the global.yml overrides not being used obviously.)

- Safemode is not thrown when the database cannot be loaded.

  • To achieve safemode I had to change the code to return false in the loadDatabase(String) method.
  • Towny did not enable at all, making greifing possible.

src/com/palmergames/bukkit/towny/Towny.java Outdated Show resolved Hide resolved
Articdive and others added 5 commits September 24, 2021 10:59
- TIE-specific messages not being logged to console.
- Skip changelog/updatingconfigversion when we're isError()'d.
- Javadoc not building.
- Specific TIEs for loading/saving database vs loading/saving
database.yml file.
- A typo.
@LlmDl LlmDl merged commit 72f0189 into master Sep 29, 2021
@LlmDl LlmDl deleted the improve-logging branch September 29, 2021 19:52
@LlmDl LlmDl restored the improve-logging branch September 29, 2021 20:14
LlmDl added a commit that referenced this pull request Sep 29, 2021
with PR #5314.
  - Improve startup logging/initialization, courtesy of ArticDive &
LlmDl with PR #5299.
@Articdive Articdive deleted the improve-logging branch October 1, 2021 19:33
@LlmDl LlmDl modified the milestones: 0.97.2.0, 0.97.3.0 Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants