Skip to content

feat(cli): update argument handler to enable local analysis with docker#48

Merged
PaulPHPE merged 11 commits intomainfrom
feat/cli/enable-local-analysis
Feb 26, 2024
Merged

feat(cli): update argument handler to enable local analysis with docker#48
PaulPHPE merged 11 commits intomainfrom
feat/cli/enable-local-analysis

Conversation

@PaulPHPE
Copy link
Copy Markdown
Collaborator

Modify cli argument handler to transform url with "localhost" hostname to url with "host.docker.internal".
Modify cli README to add the --add-host for linux users
Modify test arguments handler to verify the transformation of localhost to host.docker.internal

close #38

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 13, 2024

Coverage PR 48

Branch coverage •
FileStmtsMissCoverMissing
bases/ecoindex/cli
   arguments_handler.py671282%40–45, 47–48, 54–56, 58
   helper.py19668%16, 23–26, 28
components/ecoindex/config
   settings.py200100% 
components/ecoindex/models
   __init__.py70100% 
   cli.py40100% 
TOTAL65421167% 

@PaulPHPE
Copy link
Copy Markdown
Collaborator Author

@CodiumAI-Agent /describe

@vvatelot
Copy link
Copy Markdown
Member

Hello @PaulPHPE thanks for this PR!
It seems great. The only concern I have is that if a user runs the CLI directly with Python and not through Docker, it will also rewrite the localhost urls...
Maybe we can add an env variable in the docker image so that we take it into account to detect that we are in a docker container context? What do you think?

@PaulPHPE
Copy link
Copy Markdown
Collaborator Author

Hello @vvatelot !
Yes indeed, I hadn't thought of that, I am on it !

@PaulPHPE
Copy link
Copy Markdown
Collaborator Author

Hello @vvatelot !

First time working with Code QL for me, I am not sure what failed here and how to deal with it ?

Comment thread bases/ecoindex/cli/arguments_handler.py Outdated
Comment thread projects/ecoindex_cli/README.md Outdated
Comment thread bases/ecoindex/cli/arguments_handler.py Outdated
@vvatelot
Copy link
Copy Markdown
Member

Hello @vvatelot !

First time working with Code QL for me, I am not sure what failed here and how to deal with it ?

It seems there is an issue with CodeQL itself... I tried to re-launch the check, but still waiting to start...

Copy link
Copy Markdown
Member

@vvatelot vvatelot left a comment

Choose a reason for hiding this comment

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

If you want to build the image for mac to test it, you can rebase your branch on main and then try to launch the action https://github.com/cnumr/ecoindex_python_fullstack/actions/workflows/build_publish_cli.yml targeting your branch, with a bump of type preminor

Comment thread components/ecoindex/config/settings.py Outdated
Comment thread projects/ecoindex_cli/README.md Outdated
@PaulPHPE PaulPHPE force-pushed the feat/cli/enable-local-analysis branch from 2f236a9 to a3b3d0e Compare February 15, 2024 15:41
Comment thread components/ecoindex/config/settings.py Outdated
Comment thread bases/ecoindex/cli/arguments_handler.py
Comment thread bases/ecoindex/cli/arguments_handler.py Outdated
@PaulPHPE PaulPHPE force-pushed the feat/cli/enable-local-analysis branch from 6ad1987 to 32c5035 Compare February 20, 2024 17:04
@PaulPHPE PaulPHPE self-assigned this Feb 20, 2024
@PaulPHPE PaulPHPE merged commit a1c290f into main Feb 26, 2024
@PaulPHPE PaulPHPE deleted the feat/cli/enable-local-analysis branch February 26, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Local analyzis with CLI not working

2 participants