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

Define boundaries between Python modules #521

Closed
11 tasks
agateau-gg opened this issue May 5, 2023 · 0 comments
Closed
11 tasks

Define boundaries between Python modules #521

agateau-gg opened this issue May 5, 2023 · 0 comments
Labels
status:confirmed This issue has been reviewed and confirmed type:techdebt Fix non-optimal code

Comments

@agateau-gg
Copy link
Collaborator

agateau-gg commented May 5, 2023

Goal

To help keeping the code-base manageable, we need clearer boundaries between the various Python modules inside GGShield.

The proposed architecture is the following:

 Command layer   | cmd.iac | cmd.secret | cmd.auth | cmd.config
---------------- | ---↓----|------↓-----|----↓-----|
 Vertical layer  |   iac   |   secret   |  auth    |
---------------- | ============================================
 Support layers  |                 scan | config
                 | -----------------↓-------↓------------------
                 |                    core

A module from the command layer can only import code from its matching vertical layer (cmd.iac can import from iac but not from secret or auth). Modules from the command and vertical layers can import from any support layer modules.

Current problems and steps to fix them

  • core.config.user_config imports iac to verify IaC policy syntax
    • Move IaC policy syntax code to core.config.user_config
  • core.oauth imports config
    • Move core.oauth to auth (new module)
  • core.check_updates imports config.utils
    • Move core.config.utils.*_yaml_dict() functions from core.config.utils to a new core.yaml_utils module
  • core.client imports config
    • Move core.client to core.config

Next steps

  • Move core.config to config
  • Document the final architecture in doc/dev/architecture.md
  • Setup import-linter to enforce boundaries
@agateau-gg agateau-gg added status:confirmed This issue has been reviewed and confirmed type:techdebt Fix non-optimal code labels May 5, 2023
fbochu added a commit that referenced this issue Aug 21, 2023
This is one of the step needed to regroup all verticals into one
package and simplify the identification of the verticals layer as
defined in #521.

refs: #521
fbochu added a commit that referenced this issue Aug 21, 2023
This is one of the step needed to regroup all verticals into one
package and simplify the identification of the verticals layer as
defined in #521.

refs: #521
fbochu added a commit that referenced this issue Aug 21, 2023
This is one of the step needed to regroup all verticals into one
package and simplify the identification of the verticals layer as
defined in #521.

refs: #521
fbochu added a commit that referenced this issue Aug 21, 2023
This is one of the step needed to regroup all verticals into one
package and simplify the identification of the verticals layer as
defined in #521.

refs: #521
fbochu added a commit that referenced this issue Aug 22, 2023
This will allow to easily distinct between real commands and utils. And
eventually to enforce more easily the independence of commands' code.

refs: #521
fbochu added a commit that referenced this issue Aug 22, 2023
The hmsl quota command should not import code from the quota command.
This is needed to enforce boundaries between module as planned in #521.

refs #521.
fbochu added a commit that referenced this issue Aug 22, 2023
To enforce module boundaries, we need to move some config related
function in the `core.config` module.

refs #521
fbochu added a commit that referenced this issue Aug 22, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 23, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 23, 2023
This will allow to easily distinct between real commands and utils. And
eventually to enforce more easily the independence of commands' code.

refs: #521
fbochu added a commit that referenced this issue Aug 23, 2023
The hmsl quota command should not import code from the quota command.
This is needed to enforce boundaries between module as planned in #521.

refs #521.
fbochu added a commit that referenced this issue Aug 23, 2023
To enforce module boundaries, we need to move some config related
function in the `core.config` module.

refs #521
fbochu added a commit that referenced this issue Aug 23, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 23, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 23, 2023
This will allow to easily distinct between real commands and utils. And
eventually to enforce more easily the independence of commands' code.

refs: #521
fbochu added a commit that referenced this issue Aug 23, 2023
The hmsl quota command should not import code from the quota command.
This is needed to enforce boundaries between module as planned in #521.

refs #521.
fbochu added a commit that referenced this issue Aug 23, 2023
To enforce module boundaries, we need to move some config related
function in the `core.config` module.

refs #521
fbochu added a commit that referenced this issue Aug 23, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
This will allow to easily distinct between real commands and utils. And
eventually to enforce more easily the independence of commands' code.

refs: #521
fbochu added a commit that referenced this issue Aug 24, 2023
The hmsl quota command should not import code from the quota command.
This is needed to enforce boundaries between module as planned in #521.

refs #521.
fbochu added a commit that referenced this issue Aug 24, 2023
This will allow to easily distinct between real commands and utils. And
eventually to enforce more easily the independence of commands' code.

refs: #521
fbochu added a commit that referenced this issue Aug 24, 2023
The hmsl quota command should not import code from the quota command.
This is needed to enforce boundaries between module as planned in #521.

refs #521.
fbochu added a commit that referenced this issue Aug 24, 2023
To enforce module boundaries, we need to move some config related
function in the `core.config` module.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
This will simplify our import linter rules and allow to debug ggshield
more easily using a debugger.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
The list of extensions that are considered as binary files is not
specific to ggshield and should be in the `utils` packages according to
the new architecture.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
This function is a file related util.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
The functions inside the module are not ggshield specific and should be
in the ggshield.utils module according to the architecture described in
 #521.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
The functions inside the module are not ggshield specific and should be
in the ggshield.utils module according to the architecture described in
 #521.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
The functions inside the module are not ggshield specific and should be
in the ggshield.utils module according to the architecture described in
 #521.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
Ideally, click should only be used in the `cmd` layers and in
`utils.click`. But for now, this would required too much work to do so.
So we will just add it to the bottom layer to avoid that `utils` use it.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
The functions inside the module are not ggshield specific and should be
in the ggshield.utils module according to the architecture described in
 #521.

refs #521
fbochu added a commit that referenced this issue Aug 24, 2023
Ideally, click should only be used in the `cmd` layers and in
`utils.click`. But for now, this would required too much work to do so.
So we will just add it to the bottom layer to avoid that `utils` use it.

refs #521
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
This will allow to easily distinct between real commands and utils. And
eventually to enforce more easily the independence of commands' code.

refs: #521
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
The hmsl quota command should not import code from the quota command.
This is needed to enforce boundaries between module as planned in #521.

refs #521.
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
To enforce module boundaries, we need to move some config related
function in the `core.config` module.

refs #521
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
Import linter will allow us to maintain boundaries between our modules.

This is the first version of the configuration that only configure
boundaries between `cmd` and `verticals` modules and `core`, `utils` and
`pygitguardian`. The `core` module configuration  will be set afterward.

refs #521
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
This will simplify our import linter rules and allow to debug ggshield
more easily using a debugger.

refs #521
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
The list of extensions that are considered as binary files is not
specific to ggshield and should be in the `utils` packages according to
the new architecture.

refs #521
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
The functions inside the module are not ggshield specific and should be
in the ggshield.utils module according to the architecture described in
 #521.

refs #521
amascia-gg pushed a commit that referenced this issue Sep 4, 2023
Ideally, click should only be used in the `cmd` layers and in
`utils.click`. But for now, this would required too much work to do so.
So we will just add it to the bottom layer to avoid that `utils` use it.

refs #521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:confirmed This issue has been reviewed and confirmed type:techdebt Fix non-optimal code
Projects
None yet
Development

No branches or pull requests

1 participant