Skip to content

Conversation

@andrross
Copy link
Member

@andrross andrross commented Apr 9, 2025

The AgentPolicy class is needed at both compile time and runtime.

Check List

  • Functionality includes testing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

The AgentPolicy class is needed at both compile time and runtime.

Signed-off-by: Andrew Ross <andrross@amazon.com>
@msfroh
Copy link
Contributor

msfroh commented Apr 9, 2025

I think we need to fix the ide.gradle file as well, since it tries to reference the :libs:agent-sm jar, which may not exist if a command-line build hasn't happened yet.

Copy link
Contributor

@reta reta left a comment

Choose a reason for hiding this comment

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

@andrross we cannot do that: the :libs:agent-sm:bootstrap is shared between agent and application, but it must reside where the agent is (see please https://github.com/opensearch-project/OpenSearch/blob/main/libs/agent-sm/agent/build.gradle#L32C1-L32C50). If we do that, the agent will not see any policy. I am submitting a fix shortly.

@reta
Copy link
Contributor

reta commented Apr 9, 2025

I think we need to fix the ide.gradle file as well, since it tries to reference the :libs:agent-sm jar, which may not exist if a command-line build hasn't happened yet.

Shouldn't Gradle trigger a build since it depends on the task in :libs:agent-sm project?

@github-actions
Copy link
Contributor

github-actions bot commented Apr 9, 2025

❕ Gradle check result for 6df5d95: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@codecov
Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.36%. Comparing base (7b6108b) to head (6df5d95).
Report is 16 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #17871      +/-   ##
============================================
- Coverage     72.44%   72.36%   -0.09%     
- Complexity    66483    66497      +14     
============================================
  Files          5409     5409              
  Lines        308282   308258      -24     
  Branches      44759    44749      -10     
============================================
- Hits         223344   223076     -268     
- Misses        66608    66960     +352     
+ Partials      18330    18222     -108     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@msfroh
Copy link
Contributor

msfroh commented Apr 10, 2025

I think we need to fix the ide.gradle file as well, since it tries to reference the :libs:agent-sm jar, which may not exist if a command-line build hasn't happened yet.

Shouldn't Gradle trigger a build since it depends on the task in :libs:agent-sm project?

My experience today was that IntelliJ tried reading ide.gradle first. It couldn't find the jar, so it failed quickly. When I did a ./gradlew :libs:opensearch-agent-sm:assemble from the command line, it created the jar and IntelliJ was subsequently happy.

@reta
Copy link
Contributor

reta commented Apr 10, 2025

@andrross just to give you an idea, we could definitely improve that, but here is how we would need to approach plugins: opensearch-project/custom-codecs#235, still working to get the builds green but the idea is there, thanks!

@peterzhuamazon
Copy link
Member

@andrross just to give you an idea, we could definitely improve that, but here is how we would need to approach plugins: opensearch-project/custom-codecs#235, still working to get the builds green but the idea is there, thanks!

So does this needs to be fixed on individual plugin? or in core?

Thanks.

@peterzhuamazon
Copy link
Member

@reta
Copy link
Contributor

reta commented Apr 10, 2025

So does this needs to be fixed on individual plugin? or in core?

I think we could improve core build tooling to significantly simplify changes, but not sure yet we could eliminate it 100%

@andrross andrross closed this Apr 10, 2025
@msfroh
Copy link
Contributor

msfroh commented Apr 10, 2025

I think we need to fix the ide.gradle file as well, since it tries to reference the :libs:agent-sm jar, which may not exist if a command-line build hasn't happened yet.

Shouldn't Gradle trigger a build since it depends on the task in :libs:agent-sm project?

My experience today was that IntelliJ tried reading ide.gradle first. It couldn't find the jar, so it failed quickly. When I did a ./gradlew :libs:opensearch-agent-sm:assemble from the command line, it created the jar and IntelliJ was subsequently happy.

Actually, i was wrong -- running the command line build somehow caused IntelliJ to "keep going" after failing on the ide.gradle step, but it eventually ended up hanging on "Importing OpenSearch", locking up my laptop, requiring a hard restart.

I commented out the vmParameters += ' -javaagent:' + project(':libs:agent-sm:agent').jar.archiveFile.get() line and it managed to import the project.

@harshavamsi
Copy link
Contributor

I think we need to fix the ide.gradle file as well, since it tries to reference the :libs:agent-sm jar, which may not exist if a command-line build hasn't happened yet.

Shouldn't Gradle trigger a build since it depends on the task in :libs:agent-sm project?

My experience today was that IntelliJ tried reading ide.gradle first. It couldn't find the jar, so it failed quickly. When I did a ./gradlew :libs:opensearch-agent-sm:assemble from the command line, it created the jar and IntelliJ was subsequently happy.

Actually, i was wrong -- running the command line build somehow caused IntelliJ to "keep going" after failing on the ide.gradle step, but it eventually ended up hanging on "Importing OpenSearch", locking up my laptop, requiring a hard restart.

I commented out the vmParameters += ' -javaagent:' + project(':libs:agent-sm:agent').jar.archiveFile.get() line and it managed to import the project.

Agreed, same experience @msfroh

@msfroh
Copy link
Contributor

msfroh commented Apr 10, 2025

Ahh -- here's why it hangs: Kotlin/kotlinx-atomicfu#497

@msfroh
Copy link
Contributor

msfroh commented Apr 10, 2025

With some help from Claude, I was able to fix it by modifying the ide.gradle file to force that piece to kick in after the :libs:opensearch-agent-sm:agent project has been evaluated:

            project(':libs:agent-sm:agent').afterEvaluate { agentProject ->
              vmParameters = '-ea -Djava.locale.providers=SPI,CLDR'
              vmParameters += ' -javaagent:' + agentProject.tasks.jar.archiveFile.get()
            }

@reta
Copy link
Contributor

reta commented Apr 10, 2025

With some help from Claude, I was able to fix it by modifying the ide.gradle file to force that piece to kick in after the :libs:opensearch-agent-sm:agent project has been evaluated:

Could you please submit a pull request for it @msfroh ? Thank you!

@msfroh
Copy link
Contributor

msfroh commented Apr 10, 2025

With some help from Claude, I was able to fix it by modifying the ide.gradle file to force that piece to kick in after the :libs:opensearch-agent-sm:agent project has been evaluated:

Could you please submit a pull request for it @msfroh ? Thank you!

Done: #17886

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.

5 participants