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

Adding support for Non-Genuine mongodb like DocumentDB or CosmosDB #181 #182

Closed
wants to merge 37 commits into from
Closed

Conversation

riderx
Copy link

@riderx riderx commented Jul 9, 2020

Change Summary

After tried to create a specific component @piotch suggested to just doit in the mongoDB connector .
So now i have add a function to detect Non-Genuine MongoDB .

To detect Non-Genuine MongoDB i check if storageEngines is present in server_info, if not it look we are in presence of

I used the same commands as mongo compass L113 :
https://github.com/mongodb-js/data-service/blob/master/lib/instance-detail-helper.js

For Non-Genuine MongoDB, i completly removed the total count in get_slice function
it's always set as MAX_COUNTED_ROWS
Since it was already capped at this number, and without $facet it's not possible to aggregate this data in the same request.

It's tested on our current instance on AWS.

Related issue number

#181

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable

@piotch
Copy link
Contributor

piotch commented Jul 9, 2020

Merci! 🙏

This creates a lot of duplicated code n'est-ce pas ? Could you not import all that you need from the mongo connector and rewrite only the bits which are different ?

Testing : if you do what I suggested above the only thing you have to test is the re-written code. Which you can test against mongo (juste like we do in the mongo connector tests).

@riderx
Copy link
Author

riderx commented Jul 10, 2020

Good idea ! i will work on it !

@riderx riderx changed the title Adding support for DocumentDB with degraded mode #181 Adding support for Non-Genuine mongodb like DocumentDB or CosmosDB #181 Jul 13, 2020
@BobCashStory
Copy link

@piotch ok for you now ?

@@ -206,6 +206,28 @@ def get_df(self, data_source, permissions=None):
data_source.query = apply_permissions(data_source.query, permissions)
return self._retrieve_data(data_source)

def is_non_geniune(self):
Copy link
Member

Choose a reason for hiding this comment

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

s/geniune/genuine/g :)

Copy link
Author

Choose a reason for hiding this comment

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

Done

@fspot
Copy link
Member

fspot commented Aug 17, 2020

Closing for now because this adds an overhead of 2 requests at each call to get_slice method, which means at each data request on toucan's api using mongo connector (as this connector is used both by external and internal mongo).
If the need for this option raises again, we could use a flag on MongoConnector to activate this check.

E.g. :

check_non_genuine: Optional[bool] = Field(
    None,
    description='Checks wether the mongo server is genuine or not (e.g DocumentDB, CosmosDB)'
)

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.

4 participants