-
Notifications
You must be signed in to change notification settings - Fork 430
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
Update "PythonProjectResource" to "PythonAppResource" #5759
Conversation
/cc @aaronpowell |
src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
@@ -1,6 +1,6 @@ | |||
#nullable enable | |||
Aspire.Hosting.Python.PythonProjectResource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joperezr - do you know why all these APIs are still under "Unshipped" when we released an 8.2 version of these APIs?
The Shipped file says:
#nullable enable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah I was going to ask that lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joperezr did we update the shipped/unshipped APIs files after shipping 8.2?
Ok so I didn't change the test syntax and now its failing but there was some weird spacing so maybe thats it? @joperezr pls halp |
GREAT NEWS these test failures aren't mine!!! |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@radical the tests have gone bad again. Cosmos, Elastic search and timeouts all over. |
re:Elasticsearch, the tests need to be looked at properly. I see bunch of failures like:
Maybe this should have failed the test, but it seems to keep going. re:cosmos, that's always an issue :/ cc @eerhardt One of the reasons for timeouts is that sometimes you have lot of docker images being downloaded and increases the time taken per test! We need to figure out a solution for this one. cc @joperezr |
re:elasticsearch, towards the end you get:
.. but we keep running with rns. We should fail operations in that. |
That looks like dcp crashed or stopped running or something. |
@radical that error is the same as the one from this issue. cc @karolz-ms |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -1 +1,12 @@ | |||
#nullable enable | |||
Aspire.Hosting.Python.PythonAppResource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be in the shipped file should it as you're adding it in this PR?
I will work with @radical to see if we can narrow down these |
Description
For .NET 9, we should obsolete
AddPythonProject
and replace it withAddPythonApp
to align withAddNodeApp
etc. the "project" term should be reserved for .NET projects, ie with a .csproj, to keep the differences between a "ProjectResource" and "ExecutibleResource" clear.Notes -
I fully expect there to be edits on this since it's the first time I'm touching an Aspire Integration sooooooooo go nuts!
Fixes #5020 (in the opposite way as requested, but per discussion with @aaronpowell and @DamianEdwards this makes more sense)
Checklist
<remarks />
and<code />
elements on your triple slash comments?Microsoft Reviewers: Open in CodeFlow