Skip to content

Add --no-port-detection flag to compose command#19

Merged
thepetk merged 12 commits intodevfile:mainfrom
thepetk:feature/implement_no_port_detection_option
Aug 22, 2023
Merged

Add --no-port-detection flag to compose command#19
thepetk merged 12 commits intodevfile:mainfrom
thepetk:feature/implement_no_port_detection_option

Conversation

@thepetk
Copy link
Contributor

@thepetk thepetk commented Aug 10, 2023

What does this PR do?

This PR is part of the devfile/api#1153 EPIC and introduces a new optimization to the port detection logic. The new flag we are creating on the component command is called no-port-detection/n and simply skips the port detection part of the command.

In order to do that upon getPortDetectionStrategy func of package component it checks if the global var noPortDetection is true. If yes it applies an early return with an empty model.PortDetectionAlgorithm list.

Test cases have been added inside the new component_test.go file also updates have been made to the documentation.

As mentioned inside the proposal of the 1153 EPIC, we also add deprecation warning for the usage of port detection strategies.

Which issue(s) does this PR fix

fixes devfile/api#1150

PR acceptance criteria

Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.

  • Unit/Functional tests

  • Documentation

How to test changes / Special notes to the reviewer

I've calculated the time of execution for the new feature by running alizer against https://github.com/openshift/hypershift

With port detection:

$ time ./alizer component ~/github/alizer-tests/hypershift/
...
0m2,470s

Without port detection:

$ time ./alizer component --no-port-detection ~/github/alizer-tests/hypershift/
...
0m0,154s

Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
@thepetk thepetk requested a review from mike-hoang August 10, 2023 12:28
@thepetk thepetk self-assigned this Aug 10, 2023
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Patch coverage: 80.00% and project coverage change: -0.31% ⚠️

Comparison is base (45e6eac) 66.56% compared to head (635a4a3) 66.26%.
Report is 1 commits behind head on main.

❗ Current head 635a4a3 differs from pull request most recent head 7f54f9d. Consider uploading reports for the commit 7f54f9d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #19      +/-   ##
==========================================
- Coverage   66.56%   66.26%   -0.31%     
==========================================
  Files          10       11       +1     
  Lines        1358     1405      +47     
==========================================
+ Hits          904      931      +27     
- Misses        394      414      +20     
  Partials       60       60              
Files Changed Coverage Δ
pkg/cli/component/component.go 18.18% <71.42%> (ø)
pkg/apis/recognizer/component_recognizer.go 71.61% <100.00%> (+7.23%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
@openshift-ci
Copy link

openshift-ci bot commented Aug 15, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mike-hoang, thepetk

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: thepetk <thepetk@gmail.com>
@openshift-ci openshift-ci bot removed the lgtm label Aug 21, 2023
@openshift-ci
Copy link

openshift-ci bot commented Aug 21, 2023

New changes are detected. LGTM label has been removed.

Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
@thepetk thepetk merged commit 27607db into devfile:main Aug 22, 2023
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.

Implement Alizer --no-port-detection option

2 participants