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

Added samples for App Engine Standard Environment #579

Closed
wants to merge 2 commits into from

Conversation

michaelawyu
Copy link
Contributor

No description provided.

@michaelawyu michaelawyu requested a review from ace-n March 14, 2018 01:14
@ace-n
Copy link
Contributor

ace-n commented Mar 28, 2018

Per our conversation just now - if there aren't too many changes between the Standard and Flex samples, we should change the Flex samples to support Standard as well.

My thoughts: we can include the Standard-specific files in the same directory, e.g app.flex.yaml and app.standard.yaml and tell people to rename them accordingly in the docs. @michaelawyu sound good to you, or would that be too much confusion for users?

@RichieEscarez
Copy link

I would like to see a stand alone folder for Node.js standard only.

A few concerns about a shared sample folder:

  • Renaming all the app.yaml files has doc impact across the both the Node.js doc sets in the library (need to revisit each doc, determine what change is required (what steps to add), find the bandwidth, and then change and test it all).

  • For our Quickstarts, this also injects additional unnecessary steps. These QS should remain a simple first-touch, immediate success, low-overhead experience -> "download and run deploy command"(adding more steps to configuring a file detracts from that experience).

  • Renaming all the app.yaml files will also break the 'gcloud app deploy' command, which automatically locates the app.yaml files in the directory from which it is run (renaming it would force users to specify the filenames).

@ace-n
Copy link
Contributor

ace-n commented Mar 29, 2018

  1. I agree that combining the files will take more time initially - but I feel like that initial investment will pay off time-wise in the long run due to reduced maintenance and testing time-costs.

  2. I'd argue the additional time required to move/rename a file is minimal (O(a few seconds)) relative to the overall time users spend completing the tutorial. (Do we have data about whether said few seconds matters [not] to users when they're running through quickstarts?)

At the risk of overcomplicating things: one potential compromise (that I'm not entirely sold on yet) is to have a "master" set of files for both platforms (that isn't shown in the docs), and then use a (bash?) script to generate platform-specific versions. That way we don't have to manually update multiple files whenever we want to make changes.

@ace-n
Copy link
Contributor

ace-n commented Mar 29, 2018

Also - per the gcloud docs, it looks like you can specify the YAML file(s) to deploy. So we might be able to specify the appropriate deployment command directly in our docs, e.g: gcloud app deploy app.standard.yaml.

@RichieEscarez
Copy link

Yes, gcloud app deploy ANYFILENAMEHERE.yaml works. One issue here is that the video thats in the works mentions to simply run "gcloud app deploy".

There are approximately 28 files in the flexible environment that use the _github_include and would take time to edit and test. Perhaps we maintain simplicity for standard and leave those files as app.yaml but for any flexible environment files, rename those to app.flexible.yaml?

I realize that would leave it open to issues with Flex users just running 'gcloud app deploy' but the flexible docs would change all the copy/paste commands to "gcloud app deploy app.flexible.yaml", so it would ultimately be user error for not reading?

@ace-n
Copy link
Contributor

ace-n commented Mar 29, 2018

How hard would it be to change the video? I'd rather have such user error end in a clear "halt and catch fire" case (such as gcloud not finding the specified app.yaml file) than a "silent failure" case where the user accidentally deploys the wrong kind of app.

@steren
Copy link
Contributor

steren commented Mar 29, 2018

For quickstarts, it should be a goal that developers don't have to rename files and can just type gcloud app deploy (without specifying a .yaml file).

For other docs samples, I think it's OK to ask to use gcloud app deploy <file>.

Now, do we do:

  1. app.yaml + app.flexible.yaml
  2. app.standard.yaml + app.flexible.yaml

I have a preference for 1.

@RichieEscarez
Copy link

Ultimately Im advocating for our users most optimal experience. Our Quickstart is suppose to be that hook, "all i do is run gcloud app deploy and it works".

For this new language, we are changing the conventions we have been pushing since we started using gcloud app deploy, which might catch some users off guard.

We also dont have a app.standard.yaml task topic nor reference topic, they all refer to "app.yaml". In our advanced tasks, we do introduce the concept of multiple services and uniquely named app.yaml files but thats not a new user concept and adds unwanted cognitive load for a new user.

What are the trade offs?

Editing the 28 flex files and revising the new standard files will take some time because of all the extra language around explaining why these files are named differently (not app.yaml).

Regarding maintaining the node.js code samples, what has the overhead looked like over the last 2+ years since the flex docs went out? Im wondering what how much maintenance is required if there are two folders for Node.js (ie. what is the trade off for better UX vs. easier file handling)?

grayside pushed a commit that referenced this pull request Nov 16, 2022
grayside pushed a commit that referenced this pull request Nov 16, 2022
grayside pushed a commit that referenced this pull request Nov 16, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
grayside pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
ahrarmonsur pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 17, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
ace-n pushed a commit that referenced this pull request Nov 17, 2022
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
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.

4 participants