Skip to content
This repository has been archived by the owner on Feb 7, 2024. It is now read-only.

Throw exception when app.yaml doesn't exist #451

Closed

Conversation

cxorm
Copy link

@cxorm cxorm commented Sep 11, 2021

Fixes GoogleCloudPlatform/appengine-plugins#990

What changes were proposed in this pull request ?

The Stager Interface only check appengine-web.xml, and it would work when both app.yaml and appengine-web.xml are missing.

This patch verified the existence of app.yaml in AppYamlStager#stage().

PS. Let me steal the idea of snippet in AppDeployer : )

How was this patch tested ?

Just touch new unit test. (Feel free to correct me if I miss something.)

@google-cla

This comment has been minimized.

@cxorm
Copy link
Author

cxorm commented Sep 11, 2021

@googlebot I signed it!

testStager = new AppYamlStager(stageMojo, configBuilder);

try {
testStager.stage();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should have Assert.fail().

@chanseokoh
Copy link
Contributor

Actually, reading GoogleCloudPlatform/appengine-plugins#990, I think it's better to check the existence of either app.yaml or appengine-web.xml here and throw an error message, rather than assuming it's an app.yaml-based project and have the AppYamlStager throw an error?

@chanseokoh
Copy link
Contributor

Oh, maybe the deployer is not involved when only staging? But I think it's still better to check early for the existence of an either file that can show a clear error message than falling through the app.yaml-based workflow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misleading log message when app.yaml and appengine-web.xml file are missing
4 participants