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 database API endpoints to retrieve the list of databases. #30

Merged
merged 3 commits into from
Nov 11, 2019

Conversation

DiscoPYF
Copy link
Collaborator

#29

@DiscoPYF
Copy link
Collaborator Author

DiscoPYF commented Nov 10, 2019

I noticed that the properties in DeleteDatabaseResult and PostDatabaseResult do not conform with what is returned by ArangoDB. I left them as is in this PR and will create another issue to address that. Let me know if you think otherwise.

The examples listed in the specs:

Creating a database named example.

{ 
  "name" : "example" 
}
EOF

HTTP/1.1 Created
content-type: application/json; charset=utf-8
x-content-type-options: nosniff

{ 
  "error" : false, 
  "code" : 201, 
  "result" : true 
}

Creating a database named mydb with two users.

shell> curl -X POST --header 'accept: application/json' --data-binary @- --dump - http://localhost:8529/_api/database <<EOF
{ 
  "name" : "mydb", 
  "users" : [ 
    { 
      "username" : "admin", 
      "passwd" : "secret", 
      "active" : true 
    }, 
    { 
      "username" : "tester", 
      "passwd" : "test001", 
      "active" : false 
    } 
  ] 
}
EOF

HTTP/1.1 Created
content-type: application/json; charset=utf-8
x-content-type-options: nosniff

{ 
  "error" : false, 
  "code" : 201, 
  "result" : true 
}

- [ ] GET/_api/database List of databases
- [ ] POST/_api/database Create database
- [X] GET/_api/database List of databases
- [X] POST/_api/database Create database
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have tests yet for Create database?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll answer my own question - as far as I can tell, we haven't done the proper implementation for Create database yet so we shouldn't check it off in the task list yet. Can you uncheck it for now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep I will do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

While we do not have an explicit test for create database, most (if not all) the per-test-class fixtures use the method the create a test database.

Copy link
Collaborator

@rossmills99 rossmills99 Nov 11, 2019

Choose a reason for hiding this comment

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

Yes, the criteria for marking a task complete is that we have implemented both the endpoint and tests of the endpoint. Those methods existed for use in other tests but there are no tests explicitly for them yet.

@rossmills99
Copy link
Collaborator

I noticed that the properties in DeleteDatabaseResult and PostDatabaseResult do not conform with what is returned by ArangoDB. I left them as is in this PR and will create another issue to address that. Let me know if you think otherwise.

The examples listed in the specs:

Creating a database named example.

{ 
  "name" : "example" 
}
EOF

HTTP/1.1 Created
content-type: application/json; charset=utf-8
x-content-type-options: nosniff

{ 
  "error" : false, 
  "code" : 201, 
  "result" : true 
}

Creating a database named mydb with two users.

shell> curl -X POST --header 'accept: application/json' --data-binary @- --dump - http://localhost:8529/_api/database <<EOF
{ 
  "name" : "mydb", 
  "users" : [ 
    { 
      "username" : "admin", 
      "passwd" : "secret", 
      "active" : true 
    }, 
    { 
      "username" : "tester", 
      "passwd" : "test001", 
      "active" : false 
    } 
  ] 
}
EOF

HTTP/1.1 Created
content-type: application/json; charset=utf-8
x-content-type-options: nosniff

{ 
  "error" : false, 
  "code" : 201, 
  "result" : true 
}

Yes I agree. These have so far only been implemented to help with writing tests of the other endpoints, so they aren't considered "complete" yet. We should model the complete response from ArangoDB and that can be done as part of the "proper" implementation of those endpoints (i.e. when we come to implement tests for them and tick them off in our roadmap task list)

@rossmills99 rossmills99 merged commit 5a680a7 into ArangoDB-Community:master Nov 11, 2019
@DiscoPYF DiscoPYF deleted the feature-listDatabases branch November 11, 2019 13:50
/// available for the current user.
/// </remarks>
/// <returns></returns>
public async Task<ListDatabaseResult> ListDatabasesAsync()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rossmills99 Maybe this should be named GetDatabasesAsync?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think I agree

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I will create a new issue for that.

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.

None yet

2 participants