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

Fix initialisation ordering of AppContext #692

Closed
wants to merge 1 commit into from

Conversation

treuherz
Copy link
Contributor

@treuherz treuherz commented Jun 3, 2019

Previously, initialiseInstanceAddress() was called before LOG and DEFAULT_INSTANCE_ADDRESS were initialised, but relied on both of them, so threw an NPE instead of correctly assigning the default and starting up.

Fixes #654

Previously, `initialiseInstanceAddress()` was called before `LOG` and `DEFAULT_INSTANCE_ADDRESS` were initialised, but relied on both of them, so threw an NPE instead of correctly assigning the default and starting up.

Fixes thelastpickle#654
@michaelsembwever
Copy link
Member

Thanks for the contribution @treuherz !

The patch as is won't build because it breaks our code style, specifically the 'Declaration Order' rule:

[ERROR] /home/travis/build/thelastpickle/cassandra-reaper/src/server/src/main/java/io/cassandrareaper/AppContext.java:38: Line matches the illegal pattern 'Trailing whitespace'. [Regexp]
[ERROR] /home/travis/build/thelastpickle/cassandra-reaper/src/server/src/main/java/io/cassandrareaper/AppContext.java:43:3: Variable access definition in wrong order. [DeclarationOrder]
[ERROR] /home/travis/build/thelastpickle/cassandra-reaper/src/server/src/main/java/io/cassandrareaper/AppContext.java:44:3: Variable access definition in wrong order. [DeclarationOrder]

I will put a fix based on your work in.

michaelsembwever pushed a commit that referenced this pull request Jun 4, 2019
Previously, `initialiseInstanceAddress()` was called before `LOG` and `DEFAULT_INSTANCE_ADDRESS` were initialised, but relied on both of them, so threw an NPE instead of correctly assigning the default and starting up.

Fixes #654
ref: #692
@michaelsembwever
Copy link
Member

merged with 534f5aa

@treuherz
Copy link
Contributor Author

treuherz commented Jun 4, 2019

Appreciate the quick response. My IDE was misconfigured and the checks didn't run when I thought they had, apologies for that. It looks like it might have been that same DeclarationOrder inspection that led to this bug in the first place, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception in AppContext init throws NPE
2 participants