Skip to content

Conversation

@rnc
Copy link
Collaborator

@rnc rnc commented Oct 12, 2024

…ol_version

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 12, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@codecov
Copy link

codecov bot commented Oct 12, 2024

Codecov Report

Attention: Patch coverage is 78.94737% with 20 lines in your changes missing coverage. Please review.

Project coverage is 42.04%. Comparing base (085d45b) to head (e22538f).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
...ainer/build/preprocessor/AbstractPreprocessor.java 78.57% 5 Missing and 4 partials ⚠️
pkg/reconciler/dependencybuild/dependencybuild.go 50.00% 5 Missing and 1 partial ⚠️
pkg/reconciler/dependencybuild/buildrecipeyaml.go 94.28% 2 Missing ⚠️
...hat/hacbs/container/deploy/BuildVerifyCommand.java 75.00% 1 Missing ⚠️
...hat/hacbs/container/verifier/asm/ClassVersion.java 0.00% 1 Missing ⚠️
pkg/reconciler/artifactbuild/artifactbuild.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2172      +/-   ##
============================================
+ Coverage     41.97%   42.04%   +0.06%     
- Complexity      793      798       +5     
============================================
  Files           286      286              
  Lines         13162    13151      -11     
  Branches       1388     1392       +4     
============================================
+ Hits           5525     5529       +4     
+ Misses         7006     6997       -9     
+ Partials        631      625       -6     

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

</profile>
</profiles>
</settings>
EOF

Choose a reason for hiding this comment

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

The settings.xml for Indy, I took FROM PNC and merged with your

<settings>
  <mirrors>
    <mirror>
      <id>indy-mvn</id>
      <mirrorOf>*</mirrorOf>
      <url>
        ${MVN_REPO_DEPENDENCIES_URL}
      </url>
    </mirror>
  </mirrors>

  <servers>
    <server>
      <id>indy-mvn</id>
      <configuration>
        <connectionTimeout>60000</connectionTimeout>
        <httpHeaders>
          <property>
            <name>Authorization</name>
            <value>Bearer ${MVN_TOKEN}</value>
          </property>
        </httpHeaders>
      </configuration>
    </server>
  </servers>

  <proxies>
    <proxy>
      <id>indy-http</id>
      <active>true</active>
      <protocol>http</protocol>
      <host>indy-generic-proxy</host>
      <port>80</port>
      <!-- <username>build-ADDTW3JAGHYAA+tracking</username> -->
      <username>${BUILD_ID}+tracking</username>
      <password>${MVN_TOKEN}</password>
      <nonProxyHosts>indy|localhost</nonProxyHosts>
    </proxy>
    <proxy>
      <id>indy-https</id>
      <active>true</active>
      <protocol>https</protocol>
      <host>indy-generic-proxy</host>
      <port>80</port>
      <username>${BUILD_ID}+tracking</username>
      <password>${MVN_TOKEN}</password>
      <nonProxyHosts>indy|localhost</nonProxyHosts>
    </proxy>
  </proxies>

  <profiles>
    <profile>
      <id>deployment</id>
      <properties>
        <altDeploymentRepository>
          local::file:///var/workdir/workspace/artifacts
        </altDeploymentRepository>
      </properties>
    </profile>
  </profiles>

  <activeProfiles>
    <activeProfile>deployment</activeProfile>
  </activeProfiles>

  <interactiveMode>false</interactiveMode>
  <localRepository>/tmp/.m2/repository</localRepository>

</settings>

Choose a reason for hiding this comment

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

I would bake the ${MVN_REPO_DEPENDENCIES_URL} and ${BUILD_ID} in the OCI source image as that is required for the reproducibility and leave only ${MVN_TOKEN} (probably not the best name) configurable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we need to discuss this; I'm not entirely sure I follow or what you're trying to modify?

I don't think the server needs to be configured here as its not deploying?
I'm not entirely sure about how the proxy section is used either.

Copy link

@matejonnet matejonnet Oct 13, 2024

Choose a reason for hiding this comment

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

I don't think the server needs to be configured here as its not deploying?

I need to verify this, it might be we need auth for read operation from a per-build repo too

I'm not entirely sure about how the proxy section is used either.

this is a http proxy, used by the tools that are started by maven, eg. a maven task using wget.
Similar to maven there is an http "repo" (persistent cache) per build.

I believe this is all we need for a build using Indy, as I said it's a copy form PNC configured build, except the altDeploymentRepository

@rnc rnc force-pushed the SCRIPTS branch 4 times, most recently from 5ee554b to fe13a1c Compare October 14, 2024 16:23
@rnc rnc marked this pull request as ready for review October 14, 2024 18:53
@openshift-ci openshift-ci bot requested a review from vibe13 October 14, 2024 18:53
@rnc
Copy link
Collaborator Author

rnc commented Oct 14, 2024

/retest

@rnc rnc requested a review from matejonnet October 15, 2024 07:22
@openshift-ci openshift-ci bot added the lgtm label Oct 15, 2024
@rnc rnc merged commit 61baa4c into redhat-appstudio:main Oct 15, 2024
22 checks passed
@rnc rnc deleted the SCRIPTS branch October 15, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants