Navigation Menu

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

Add Neutron agent-list #807

Merged
merged 5 commits into from Sep 19, 2016
Merged

Add Neutron agent-list #807

merged 5 commits into from Sep 19, 2016

Conversation

biogerm
Copy link

@biogerm biogerm commented Sep 13, 2016

Neutron agent-list function was supported.
Fixed a missing symbol bug from Trove merge

This change is contributed by Yin Zhang, Magnus Broman, Qin An
(@biogerm)

Neutron agent-list function was supported.
Fixed a missing symbol bug from Trove merge

This change is contributed by Yin Zhang, Magnus Broman, Qin An
(@biogerm)
@biogerm
Copy link
Author

biogerm commented Sep 14, 2016

@gondor @vinodborole @auhlig Here is the new PR. The old one was abandoned. Please take a look. Thank you!

* @param str - string to be parsed
* @return Date
*/
private Date parseDate(String str) {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Sounds like a good idea. I am now working on next PR so I will make a note and include it later.

@biogerm
Copy link
Author

biogerm commented Sep 15, 2016

Waiting for this to be merged because this commit fixes compiling issue. @gondor @vinodborole

@auhlig
Copy link
Member

auhlig commented Sep 15, 2016

@biogerm Could you address the comment and resolve the conflict?

Neutron agent-list function was supported.
Fixed a missing symbol bug from Trove merge

This change is contributed by Yin Zhang, Magnus Broman, Qin An
(@biogerm)
@biogerm
Copy link
Author

biogerm commented Sep 15, 2016

@auhlig Ah. I was thinking to update source code in the next PR but since there's conflicts again... ok I will push a new PR for this.

@biogerm
Copy link
Author

biogerm commented Sep 15, 2016

@auhlig I looked into this and here is what I found out about. Neutron Agent has a different time stamp format. It's different than other API calls say, image-list. So that's why there's a different parser just for Agent list. Should I move this parser to Parser class or just keep it here? What do you say?

@auhlig
Copy link
Member

auhlig commented Sep 15, 2016

One place for that kind of methods might be better than multiple in order to avoid unnecessary code duplication. So I would move it to the Parser class. Do you agree?

@biogerm
Copy link
Author

biogerm commented Sep 16, 2016

I'm thinking the same. Here's the new commit.

Conflicts:
	core-test/src/main/java/org/openstack4j/api/network/NetworkTests.java
	core/src/main/java/org/openstack4j/openstack/networking/domain/NeutronAgent.java
	core/src/main/java/org/openstack4j/openstack/provider/DefaultAPIProvider.java
@auhlig
Copy link
Member

auhlig commented Sep 16, 2016

Cool. Thanks for all that work @biogerm

@auhlig
Copy link
Member

auhlig commented Sep 17, 2016

Hey @biogerm,
Seems like the recent merges, also of your work in the other PRs, created some conflicts. Could you resolve them and rebase and squash your commits? Sorry for the additional work. I promise to merge this PR as soon as this is done! :)

# Conflicts:
#	core/src/main/java/org/openstack4j/api/Builders.java
#	core/src/main/java/org/openstack4j/openstack/provider/DefaultAPIProvider.java
@biogerm
Copy link
Author

biogerm commented Sep 19, 2016

@auhlig no problem. it's quick :)

@auhlig
Copy link
Member

auhlig commented Sep 19, 2016

Thanks. Merging now

@auhlig auhlig merged commit 3826c31 into ContainX:master Sep 19, 2016
@auhlig auhlig added this to the 3.0.3 Release milestone Sep 20, 2016
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.

None yet

2 participants