Skip to content

Conversation

@johnsabath
Copy link
Contributor

@johnsabath johnsabath commented Oct 24, 2022

There are some unexpected diffs due to some of the files being generated with newer versions of protoc previously, but I don't believe that the changes will have an affect on the runtime behavior. The outputs in this MR were generated by the docker build process. My hope is that these unintentional diffs will no longer be an issue once building via docker is established as the norm.

@salanki salanki requested review from arsenetar and wbrown October 24, 2022 17:09
@johnsabath johnsabath force-pushed the js/estimate-cost-operation branch from 7cb6470 to 13b8567 Compare October 25, 2022 14:40
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

service GenerationService {
rpc Generate (Request) returns (stream Answer) {};
rpc ChainGenerate (ChainRequest) returns (stream Answer) {};
rpc EstimateCost (Request) returns (EstimateCostResponse) {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to break from the Answer pattern here? I suggest against this. We have metadata around this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My initial impression was that there was a fair bit of additional risk introduced if I modified the generation logic to support a "dry run" / "cost estimation" mode where an Answer could come back without any image artifacts, but that's largely due to me being unfamiliar with the code and not having a test case safety net to lean on.

I'll take another stab at this if maintaining that pattern is the goal.

@johnsabath johnsabath closed this Nov 1, 2022
@johnsabath johnsabath deleted the js/estimate-cost-operation branch November 1, 2022 20:03
@johnsabath
Copy link
Contributor Author

New PR: #35

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.

3 participants