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.

Sign up for GitHub

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

Jump to bottom

CLOUDSTACK-10144: fix possible NPE in listVlanIpRanges #2330

Merged
merged 1 commit into from Nov 17, 2017
Merged

CLOUDSTACK-10144: fix possible NPE in listVlanIpRanges #2330

merged 1 commit into from Nov 17, 2017

Conversation

resmo
Copy link
Member

@resmo resmo commented Nov 16, 2017

Prevent NPE for querying a deleted project.

GabrielBrascher
GabrielBrascher approved these changes Nov 16, 2017
View reviewed changes
Copy link
Member

@GabrielBrascher GabrielBrascher left a comment

Choose a reason for hiding this comment

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

Code LGTM. Thanks @resmo!

@ustcweizhou
Copy link
Contributor

code LGTM.

However, could you please add some code to release vlan and ip ranges while delete a project ?
If it is not an issue anymore, please ignore my comment :)

rohityadavcloud
rohityadavcloud approved these changes Nov 16, 2017
View reviewed changes
Copy link
Member

@rohityadavcloud rohityadavcloud left a comment

Choose a reason for hiding this comment

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

LGTM

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@resmo
Copy link
Member Author

resmo commented Nov 16, 2017

rollback transaction on releasing vlan and ip ranges while delete a project was the root cause getting into this state. Unsure yet, while that happened.

However, preventing an NPE can not be wrong.

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✖debian. JID-1266

@ustcweizhou
Copy link
Contributor

@resmo agreed. let's merge this at first.

It would be nice to fix the issue you said.

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1267

@rohityadavcloud
Copy link
Member

Simple npe check, merging based to Travis and packaging validations.

@rohityadavcloud rohityadavcloud merged commit 55c059c into apache:master Nov 17, 2017
@resmo resmo deleted the fix/npe_listVlanIpRanges branch November 17, 2017 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rohityadavcloud rohityadavcloud rohityadavcloud approved these changes

@GabrielBrascher GabrielBrascher GabrielBrascher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@resmo @ustcweizhou @rohityadavcloud @blueorangutan @GabrielBrascher