bwgjoseph / mongoose-vs-ottoman

feature comparison between mongoose and ottoman
0 stars 1 forks source link

management api #111

Open bwgjoseph opened 2 years ago

bwgjoseph commented 2 years ago

Hi,

While playing around with management api such as bucket/scope/collection creation/drop, I noticed the following issues and provide some feedback.

  1. createBucket

image

CreateBucketOptions was exposed but RequestSpan isn't

image

await ottoman.bucketManager.createBucket({
    name: 'tempBucket',
    flushEnabled: false,
    ramQuotaMB: 100,
    numReplicas: 1,
    replicaIndexes: false,
    bucketType: BucketType.Couchbase,
    evictionPolicy: EvictionPolicy.ValueOnly,
    maxExpiry: 0,
    minimumDurabilityLevel: DurabilityLevel.None, // DurabilityLevel
    compressionMode: CompressionMode.Passive,
    conflictResolutionType: ConflictResolutionType.SequenceNumber,
});

Suggest to have defaults, so developer don't have to indicate all possible fields? It's the same as the UI where I just need to input name field, thus, leaving on the cardinal field like name to be non-optional

image

So if I have some code like this to init a bucket and drop right after, it won't work

await ottoman.bucketManager.createBucket({
    name: 'tempBucket',
    flushEnabled: false,
    ramQuotaMB: 100,
    numReplicas: 1,
    replicaIndexes: false,
    bucketType: BucketType.Couchbase,
    // note the fieldname conflict
    evictionPolicy: EvictionPolicy.ValueOnly,
    ejectionMethod: EvictionPolicy.ValueOnly,
    // note the fieldname conflict
    maxExpiry: 0,
    maxTTL: 0,
    // note the fieldname and type conflict
    minimumDurabilityLevel: DurabilityLevel.None, // DurabilityLevel
    durabilityMinLevel: 'none', // string
    compressionMode: CompressionMode.Passive,
    conflictResolutionType: ConflictResolutionType.SequenceNumber,
}).catch(error => {
    if (error instanceof BucketExistsError) {
        console.log(error);
    }
});

await ottoman.bucketManager.dropBucket('tempBucket');

If I run the first time, it will show

image

and keep running the same test, it will show

image

Suspect that the creation is not yet ready but I called the drop right after. But it would be weird because I have already awaited for it to be created first.

Tried to create bucket, scope and collection like such

await ottoman.bucketManager.createBucket({
    name: 'tempBucket',
    flushEnabled: false,
    ramQuotaMB: 100,
    numReplicas: 1,
    replicaIndexes: false,
    bucketType: BucketType.Couchbase,
    // note the fieldname conflict
    evictionPolicy: EvictionPolicy.ValueOnly,
    ejectionMethod: EvictionPolicy.ValueOnly,
    // note the fieldname conflict
    maxExpiry: 0,
    maxTTL: 0,
    // note the fieldname and type conflict
    minimumDurabilityLevel: DurabilityLevel.None, // DurabilityLevel
    durabilityMinLevel: 'none', // string
    compressionMode: CompressionMode.Passive,
    conflictResolutionType: ConflictResolutionType.SequenceNumber,
}).then(async () => {
    await ottoman.collectionManager.createScope('tempScope');
    await ottoman.collectionManager.createCollection({
        name: 'tempCollection',
        scopeName: 'tempScope'
    });
}).catch(async (error) => {
    if (error instanceof BucketExistsError) {
        console.log(error);
    }
});

After running it, the bucket gets created but the scope and collection does not

image


will update the list as I finds out more

gsi-alejandro commented 2 years ago

hi @bwgjoseph

  1. About couchbase types I will expose all of them scoped to a Couchbase namespace.

example to use:

import { Couchbase } from 'ottoman';
const bucketSettings: Couchbase.ICreateBucketSettings = {};

image

  1. createBucket method is async, but sadly it doesn't ensure the bucket created will be ready to use.

I need to meet with the couchbase SDK team to see how viable is to make an implementation to include the time to ensure the bucket is ready to work with.

bwgjoseph commented 2 years ago

Hello @gsi-alejandro,

Would that be different from what you exposed for the other types/interface/exception? For example, DocumentNotFoundError and other exceptions are also from Couchbase but was exposed directly through ottoman

// this
import { DocumentNotFoundError, ValidationError } from 'ottoman';
// instead of 
import { Couchbase } from 'ottoman';

Couchbase.DocumentNotFoundError

Also, for the case of this

await getDefaultInstance().cluster.query(query, { scanConsistency: QueryScanConsistency.RequestPlus });

I have to import via Couchbase directly

import { QueryScanConsistency } from 'couchbase';


Please do, otherwise, this API would be not as useful, and I can't rely on it. Do also note the other feedback regarding the default values rather than having to input all the properties.

Thanks!

gsi-alejandro commented 2 years ago

Hi @bwgjoseph

We currently are exposing some couchbase API the same way you are suggesting, but many times you or any other Ottoman's users request access to couchbase types we don't export and we can't simply export everything from couchbase at the package level to avoid name collisions.

Then the proposal is to export a Couchbase container that will ship everything from couchbase package. Allowing the access this way and ensure everything will be exported:

import { Couchbase } from 'ottoman';
const { DocumentNotFoundError } = Couchbase;

On the other hand, we can Identify the couchbase API needed by devs, check if it's possible to export it with the same name or export and alias if there is a name collision. (I don't like this solution because is prompt to generate breaking changes and aliases will be misleading for couchbase users)

bwgjoseph commented 2 years ago

So do you mean that everything will be exported via the Couchbase namespace including all those that you already have exported through ottoman right now?

The only downside I can think of is dev will lose the discoverability of the types. Meaning to say that IDE intellisense won't be able to detect and help to import. That itself is a big issue IMO.

gsi-alejandro commented 2 years ago

There will be no changes with the already exported API from couchbase to ottoman. Gradually we will go to export all couchbase types used directly by Ottoman with the dependencies types. (it's the goal)

e.g. createBucket will need BucketSettings and BucketOptions, them ottoman will provide those types directly if possible.

The Couchbase namespace will be used as a fallback for none exported types and prevent future changes without manually process by first-hand, to always make sure we are providing the types for devs to works.

This implementation avoids name collision by default, allowing us to test and adopt couchbase SDK new versions, while enhancing ottoman API.

The goal is to provide the best dev experience, exposing everything needed directly from ottoman (as you suggested), but we have to ensure it before. (we can't blindly export everything from couchbase SDK at ottoman's package level)

P.S. BucketSettings and BucketOptions will be exposed directly from ottoman.

I understand your concern and agreed with you, we are working to achieve it.

Thank you for your support and feedback.

gsi-alejandro commented 2 years ago

There will be no changes with the already exported API from couchbase to ottoman. Gradually we will go to export all couchbase types used directly by Ottoman with the dependencies types. (it's the goal)

e.g. createBucket will need BucketSettings and BucketOptions, them ottoman will provide those types directly if possible.

The Couchbase namespace will be used as a fallback for none exported types and prevent future changes without manually process by first-hand, to always make sure we are providing the types for devs to works.

This implementation avoids name collision by default, allowing us to test and adopt couchbase SDK new versions, while enhancing ottoman API.

The goal is to provide the best dev experience, exposing everything needed directly from ottoman (as you suggested), but we have to ensure it before. (we can't blindly export everything from couchbase SDK at ottoman's package level)

P.S. BucketSettings and BucketOptions will be exposed directly from ottoman.

I understand your concern and agreed with you, we are working to achieve it.

Thank you for your support and feedback.

gsi-alejandro commented 2 years ago

There will be no changes with the already exported API from couchbase to ottoman. Gradually we will go to export all couchbase types used directly by Ottoman with the dependencies types. (it's the goal)

e.g. createBucket will need BucketSettings and BucketOptions, them ottoman will provide those types directly if possible.

The Couchbase namespace will be used as a fallback for none exported types and prevent future changes without manually process by first-hand, to always make sure we are providing the types for devs to works.

This implementation avoids name collision by default, allowing us to test and adopt couchbase SDK new versions, while enhancing ottoman API.

The goal is to provide the best dev experience, exposing everything needed directly from ottoman (as you suggested), but we have to ensure it before. (we can't blindly export everything from couchbase SDK at ottoman's package level)

P.S. BucketSettings and BucketOptions will be exposed directly from ottoman.

I understand your concern and agreed with you, we are working to achieve it.

Thank you for your support and feedback.

bwgjoseph commented 2 years ago

You're right, and I agree with you too. It's a hard balance to find between providing better dx, and maintaining the types with the sdk.

I think it's something that can be slowly improve over time, and that's alright.

Thanks for taking in my feedback as well, glad that my feedback do help to build a better product too.

gsi-alejandro commented 2 years ago

hi @bwgjoseph

Tomorrow Ottoman will release the stable version 2.0.0 that will include many new types from couchbase SDK, in addition to the existing ones.

https://github.com/couchbaselabs/node-ottoman/blob/master/src/couchbase.ts