modularization: Datasets modularization pt.2

[nikpodsh]

Feature or Bugfix

• Refactoring

Detail

Refactoring of DatasetProfilingRun

Relates

• Modularization of data.all code #295 and Modularization of Datasets (backend) #412

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[dlpzx]

Why did we change the naming convention? api graphql Objects were capitalized before right?

[nikpodsh]

That's correct and if you think it should remain the existing way please let me know.
I thought that this modularization is a good opportunity to start following the python convention ( underscore instead of capitalized).
But as I said if you feel that it's not correct (or capitalized is a unique style of data.all :)) I will return it back

[dlpzx]

super small comment - I think we could standardize the names further: api.profiling, models.db.DatasetProfilingRun, services.dataset_profiling. For the handler, I think we should have a Glue client into commons and a profiling_handler, what do you think?

[nikpodsh]

About the naming: I wrote this long names like dataset_profiling_service because it allows better to navigate across files and it's solves name clashes in IDE when a lot files are opened. But as a downside the names are getting bigger. If you think that it would more beneficial to have something like services.profiling rather than services.profiling_service (and the same for the rest) I will rename it

About the handler: No, unfortunately, we can't keep Glue in common, because it has a dependency for the models of different modules. The only way to solve it is to split it and put a glue code related to modules into the modules. Otherwise, we will have a circular imports

[dlpzx]

I adding here the description of the PR just to check that I got a clear view of it:

In this PR we are extracting the Profiling piece from the Datasets. The profiling feature uses the Glue profiling job that is deployed as part of the Dataset stack. Users trigger the run of this job from the UI "profiling tab" and the results are plotted in the UI.

• db.models.specific_name are added under the modules.datasets.db.models
• In this case we have more than 1 graphql object, we are placing each in a package in the module.datasets.api directory
• moved resolvers code to the modules.datasets.services
• in this PR the handlers relative to profiling jobs have been extracted to a package inside modules called handlers, but I guess this is going to be in aws module righ @nikpodsh ?

Profiling is one feature that I see customers enabling/disabling from Datasets. In case they disable it the associated Glue Job deployed in the Dataset should also not be deployed. But we can think about that once the Dataset modularization is complete

[nikpodsh]

I adding here the description of the PR just to check that I got a clear view of it:

Thanks for that :)

in this PR the handlers relative to profiling jobs have been extracted to a package inside modules called handlers, but I guess this is going to be in aws module right @nikpodsh ?

I thought about it this way:
handlers contains the code that runs in the async lambda (the one where we send a message in SQS to)
aws contains the code (clients) that sends request to AWS. it's an abstraction and API for all request to AWS. The request to AWS can be send both from async and GraphQL lambdas.

Why do we need to separate them?

1. If code for handlers and clients (aws) are in the same package then GraphQL lambda will load the code of async lambda automatically, when it's need to send a direct request.
2. It's better layer separation: let's imagine that you got a CloudWatch log that say that the parameter is missing for some request to AWS. If we have a package where we keep API to send request, we know where to look for. The same goes if the error in async lambda. :)