Skip to content

Datastore training#4

Open
Najipian wants to merge 19 commits intocamelcasetechsd:masterfrom
Najipian:datastore-training
Open

Datastore training#4
Najipian wants to merge 19 commits intocamelcasetechsd:masterfrom
Najipian:datastore-training

Conversation

@Najipian
Copy link
Contributor

No description provided.

Copy link
Member

@zhibek zhibek left a comment

Choose a reason for hiding this comment

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

@Najipian It's looking good. I've added some small review comments.

@@ -0,0 +1,6 @@
import webapp2
Copy link
Member

Choose a reason for hiding this comment

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

Remove imports that aren't required/used in this file.

import webapp2
import logging
import os
from google.appengine.ext import ndb
Copy link
Member

Choose a reason for hiding this comment

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

Remove imports that aren't used in this file

<li><a href="/examples/yaml">Yaml example</a></li>
<li><a href="/training/pipeline/5">Pipeline training</a></li>
<li><a href="/training/http/30.05/31.25">Http training </a></li>
<li><a href="/training/http/{{lat}}/{{lng}}">Http training </a></li>
Copy link
Member

Choose a reason for hiding this comment

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

Avoid trailing whitespace in links.

logging.info("DatastoreTrainingHandler")

# get entities
entities_for_example_kind = Example.query().fetch(20)
Copy link
Member

Choose a reason for hiding this comment

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

Rather than having 20 as a "magic number" lets make a constant LIST_DEFAULT_LIMIT = 20 in this file and use that instead.

<body>
<h1>Datastore Training</h1>
<dl>
{% for entity in entities %}
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a conditional so No entities found is displayed if no entities exist.

@@ -1,4 +1,3 @@
import webapp2
import logging
Copy link
Member

Choose a reason for hiding this comment

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

@Najipian Logging is still imported, but not used.

<dt><p>{{ entity.key }}</p></dt>
<dd><p>{{ entity.value }}</p></dd>
{% endfor %}

Copy link
Member

Choose a reason for hiding this comment

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

Please check redundant newlines and incorrect indentation.

import logging
import os
from google.appengine.ext import ndb

Copy link
Member

Choose a reason for hiding this comment

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

Why has an empty newline been left?

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.

2 participants