Skip to content
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

[release/8.0] AppModel API omnibus PR for GA #3755

Merged
merged 5 commits into from
Apr 19, 2024
Merged

Conversation

mitchdenny
Copy link
Member

@mitchdenny mitchdenny commented Apr 17, 2024

WithEnvironment constraint (#3750)

Customer Impact

Overload of WithEnvironment had the wrong constraint. Customers may assume that adding an environment variable these resources has an impact. It would also set them up for a break when we fix this in 8.1 so fixing it in 8.0 since it is a minor low risk change right now.

Testing

Code itself hasn't changed, just the constraint. Everything compiles including playgrounds.

Risk

Low.

Regression?

No.

AddSeq WithDataBindMount WithDataVolume (#3762)

Customer Impact

This is a breaking change. We removed the data directory argument on AddSeq and instead introduced
two extension methods named WithDataBindMount and WithDataVolume to be consistent with other container resources. We could not [Obsolete] the old method because it has an optional argument and it would introduce ambiguity to the compiler - so we had to remove it outright with this change.

Testing

Manual testing and unit testing.

Risk

Low.

Regression?

No.

Add callback to WithPgAdmin and add WithHostPort (#3801)

Customer Impact

modifies the arguments for WithPgAdmin to take a callback which allows for a greater array of customization. Usage example:

builder.AddPostgres("pgslq").WithPgAdmin(c => c.WithImageTag("8.3").WithHostPort(8999));

This is a breaking change to the WithPgAdmin API. If someone was using the port argument they'll now need to use the callback syntax. The benefit is that they can do more customizations to the container. Until this change we didn't provide a way for people to bump their container image.

Testing

Manual testing and unit testing (added a test).

Risk

Low.

Regression?

No.

Add WithDataVolume and WithDataBindMount to support AddNats (#3763)

Customer Impact

Currently there is an extension method for IResourceBuilder<NatsServerResource> called WithJetStream(...). With JetStream automatically creates a bind mount when it enables the
JetStream feature on Nats. This change separates that out so that it is consistent with other resources so we have a WithDataBindMount and WithDataVolume extension methods.

Testing

Manual testing. Unit test updated to accommodate the change.

Risk

Low.

Regression?

No.

WithRedisCommander and WithMongoExpress configContainer callbacks. (#3826)

Customer Impact

Breaking changes to the API for WithRedisCommander and WithMongoExpress. Basically adding the container configuration callback the same as was done to Postgres above. Also introduced the WithHostPort methods.

The PR also renames the Cosmos and Storage UseX methods to WithX methods for overall consistency.

Testing

Manual testing. Unit tests

Risk

Low.

Regression?

No.

Microsoft Reviewers: Open in CodeFlow

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Apr 17, 2024
@dotnet-policy-service dotnet-policy-service bot added the Servicing-consider Issue for next servicing release review label Apr 17, 2024
* Add WithDataVolume() and WithDataBindMount() APIs to Seq.
* Add callback to WithPgAdmin to allow customization.

* Fix const names.
@mitchdenny mitchdenny self-assigned this Apr 18, 2024
@mitchdenny mitchdenny changed the title [release/8.0] Final omnibus PR for GA [release/8.0] AppModel API omnibus PR for GA Apr 18, 2024
@mitchdenny mitchdenny added this to the GA milestone Apr 18, 2024
@danmoseley
Copy link
Member

Supportive of these so far. If it would help @mitchdenny we can merge these three now (if reviewed)

@danmoseley danmoseley added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 18, 2024
@danmoseley
Copy link
Member

@davidfowl OK with these?

@mitchdenny
Copy link
Member Author

I'm OK to merge them now. Better to have them in and being tested by Bala's team than sitting here doing nothing.

@mitchdenny
Copy link
Member Author

Actually going to cherry pick one more change into this one (just need a few mins).

* Fixed NATS volume mounts
@mitchdenny
Copy link
Member Author

@danmoseley this is ready now.

@RussKie RussKie enabled auto-merge (squash) April 19, 2024 02:45
@danmoseley
Copy link
Member

@davidfowl supportive? If so I'm good with it

…3826)

* Introduce callback for WithRedisCommander.
* Introduce callback for WithMongoExpress
* Introduce WithHostPort for Mongo Express, PGAdmin4, and Redis Commander
* Rename UseGatewayPort to WithGatewayPort
* Rename UseBlobPort (and friends) to WithBlobPort (and friends)
@mitchdenny
Copy link
Member Author

@davidfowl @danmoseley as discussed, added on the WithMongoExpress and WithRedisCommander container config callbacks as well as a few other tweaks for Cosmos and Storage. These are breaking API changes but these are less frequently used APIs and necessary for overall polish/consistency.

@mitchdenny mitchdenny added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Apr 19, 2024
@RussKie RussKie merged commit 23d7920 into release/8.0 Apr 19, 2024
8 checks passed
@RussKie RussKie deleted the ga-omnibus-final-pr branch April 19, 2024 14:07
@github-actions github-actions bot locked and limited conversation to collaborators May 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants