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

Move AuthRPC client inside lib/auth #44

Open
fredmaggiowski opened this issue Sep 5, 2016 · 3 comments
Open

Move AuthRPC client inside lib/auth #44

fredmaggiowski opened this issue Sep 5, 2016 · 3 comments
Assignees

Comments

@fredmaggiowski
Copy link
Member

Let's make the auth library self-contained by letting her export even the RPC Client (and avoid custom/copy-paste implementation in each package)

@fredmaggiowski fredmaggiowski added this to the Produce a basic alpha milestone Sep 5, 2016
@fredmaggiowski fredmaggiowski self-assigned this Sep 5, 2016
fredmaggiowski pushed a commit that referenced this issue Sep 6, 2016
fredmaggiowski pushed a commit that referenced this issue Sep 6, 2016
fredmaggiowski pushed a commit that referenced this issue Sep 6, 2016
…orageservice tests are failing. (TestStorageResourceFlow, TestStorageUploadResourceDuplicated, TestStorageDeleteJobDuplicated)
@fredmaggiowski
Copy link
Member Author

@dystonie

I've refactored auth and db packages and unified all the database mocks in a single one (in lib/database/mock.

Now everything compiles fine with make but the following tests in storageservice are failing:

  • TestStorageResourceFlow;
  • TestStorageUploadResourceDuplicated;
  • TestStorageDeleteJobDuplicated

I gave a look but I honestly I'm not 100% clear of what is happening there..

@dystonie
Copy link
Contributor

dystonie commented Sep 7, 2016

@FedericoMaggi

I do not agree with the aggregation you have done: database frontend are service specific and implements the minimal functions required by the specific associated service, the should be included with the service itself (also for better being able to add functionalities if required). Consider that these interfaces are not logically intended to wrap all used database capabilities.

I agreed with your idea about separating (inside a subfolder in the service dir for ex. "auth/mock") the mock implementations but I do not see the point in dividing actual database wrapping function: it makes less readable code; less maintainable and mix functionalities absolutely non related.

I would suggest restoring the previously organization and only proceed with mock separation.

@fredmaggiowski
Copy link
Member Author

To summarise our talk about this issue today:

  • lib/auth is OK
  • lib/database should be deleted and all the database.go and mock files moved to each package.

I'd consider renaming the file from database.go to something more evocative of what it really is (since it is NOT a database wrapper)

Mocks will be placed in a mock subpackage hence for example we'll have:

3nigm4/<service>/mock

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

No branches or pull requests

2 participants