data-dot-all / dataall

A modern data marketplace that makes collaboration among diverse users (like business, analysts and engineers) easier, increasing efficiency and agility in data projects on AWS.
https://data-dot-all.github.io/dataall/
Apache License 2.0
229 stars 82 forks source link

Custom confidentialty mapping should be in dataset_base instead of s3_datasets section of config.json #1429

Open TejasRGitHub opened 1 month ago

TejasRGitHub commented 1 month ago

Is your idea related to a problem? Please describe. Since confidentiality is a business classification type, it should be in the datasets_base section. With new changes in v2.6, the custom confidentiality mapping is moved to s3_dataset section

Describe the solution you'd like Change the code to get custom confidentiality mapping from datasets_base instead from s3_datasets section

P.S. Don't attach files. Please, prefer add code snippets directly in the message body.

noah-paige commented 1 month ago

Hi @TejasRGitHub - this is a good catch, agreed confidentiality and all the associated feature flags should now fall under dataset_base rather than s3_datasets with the latest refactoring in config.json (along with all subsequent areas where we read config)

Would you be willing to contribute this refactor change? If not I will also add to backlog and sync with team to see if have capacity to pick up shortly!

dlpzx commented 1 week ago

Hi @TejasRGitHub, with the latest changes in the following 2.7 release (already in main ) the resulting config.json will look like:

        "datasets_base": {
            "active": true,
            "features": {
                "share_notifications": {
                    "email": {
                        "active": false,
                        "persistent_reminders": false,
                        "parameters": {
                            "group_notifications": true
                        }
                    }
                },
                "confidentiality_dropdown" : true,
                "topics_dropdown" : true,
                "auto_approval_for_confidentiality_level" : {
                    "Unclassified" : true,
                    "Official" : true,
                    "Secret" : true
                },
                "share_expiration" : {
                    "active" : true,
                    "run_schedule" : [1, 3, 7, 14, 21, 28]
                }
            }
        },
        "s3_datasets": {
            "active": true,
            "features": {
                "file_uploads": true,
                "file_actions": true,
                "aws_actions": true,
                "preview_data": true,
                "glue_crawler": true
            }
        },
        "s3_datasets_shares": {
            "active": true
        },
        "redshift_datasets": {
            "active": true
        },
        "redshift_datasets_shares": {
            "active": true
        },

With this i think the issue is solved. Can you confirm?

noah-paige commented 1 week ago

I think the issue is that when we try to fetch custom_confidentiality_mapping from config.json sometimes the config.json path is specified as config.modules.s3_datasets.features.custom_confidentiality_mapping and not config.modules.datasets_base.features.custom_confidentiality_mapping

TejasRGitHub commented 1 week ago

I think @noah-paige's comment describes the issue I was mentioning. In which the custom confidentiality has to be refactored to point to config.modules.datasets_base.features.custom_confidentiality_mapping