More rust training content #7

brson · 2019-03-23T00:47:53Z

It's been a long time since I've submitted anything, so here's my current branch.

The main things to note here is that project 1 has more text now, and that I've removed gRPC and tokio from the plan (from discussion with Liu we agreed they were too ambitious for these 5 projects and could come in a later project).

Rendered

Tomorrow I will create an issue outlining the next steps so that others might help.

PTAL @overvenus @nolouch

brson · 2019-03-23T00:48:27Z

cc @nrc happy to hear any suggestions.

brson · 2019-03-25T01:59:28Z

I've pushed another commit that completes the text for project 1.

sticnarf · 2019-03-25T03:10:23Z

rust/CONTRIBUTING.md

+solutions - students should learn where to get the answers from the
+pre-requisites and lessons.
+
+Proect text may include inline links to pages that offer explanatios of terms


Typo.

Suggested change

Proect text may include inline links to pages that offer explanatios of terms

Project text may include inline links to pages that offer explanations of terms

sticnarf · 2019-03-25T03:13:20Z

rust/CONTRIBUTING.md

@@ -0,0 +1,89 @@
+TODO organize this
+
+Every md file has a html relative symlink to index.html:


Better to explain the reason? I first felt confused until I find out we are loading the md dynamically according to the url.

sticnarf · 2019-03-25T03:26:51Z

rust/CONTRIBUTING.md

+
+## Contributor notes
+
+- each md file needs a relative symlink to index.html


To be more clarified, let's use an html relative symlink same as the beginning.

sticnarf · 2019-03-25T03:39:20Z

rust/CONTRIBUTING.md

+
+> _Use [crates.io](https://crates.io) to find the documentation
+for the `clap` crate, and implement the command line interface
+such that the `cli_*` test cases pass._


On the rendered page, the _ in the inline code interrupts the em block. It's actually a bug of marked. Should we use html tag as a workaround or wait for them fixing it?

sticnarf · 2019-03-25T03:40:31Z

rust/projects/tools/project.md

+
+_Use [crates.io](https://crates.io) to find the documentation
+for the `clap` crate, and implement the command line interface
+such that the `cli_*` test cases pass._


sticnarf · 2019-03-25T05:30:07Z

rust/plan.md


- Lesson: Build-time Rust
+- Use hyper for synchronous networking


To my knowledge, as HTTP servers, the latest hyper and iron are all asynchronous and run over the tokio runtime. Maybe we can consider use http crate to build a synchronous HTTP server ourselves?

My fault. Only at the master branch iron uses hyper 0.12 but the released 0.6.0 version uses the synchronous hyper 0.10.

Then, how will we make it single threaded first and change it to multi-threaded in the next project? By using hyper 0.10 or iron, that's simply to set the thread num of the pool to 1 first and then make it larger. This does not make much sense.

I was under the impression hyper still offered a synchronous API, but it does not seem to.

Building a synchronous HTTP server could be interesting, requiring direct use of the std networking APIs. It's also demystifying, shows that HTTP is approachable. It's not all that practical, since most people should not implement HTTP themselves.

Another solution would be to use gRPC instead of HTTP.

I'm going to leave this an open question.

sticnarf · 2019-03-25T08:54:08Z

rust/projects/tools/project.md

 through the project, otherwise you will be distracted by the many failing tests
-that you are not currently working on fixing.
+that you have not yet fixed.

 **Question D**: even if a given test suite doesn't contain any tests, why


Suggested change

**Question D**: even if a given test suite doesn't contain any tests, why

**Question D**: Even if a given test suite doesn't contain any tests, why

sticnarf · 2019-03-25T08:57:14Z

rust/projects/tools/project.md

+
+Again, the interface for the CLI is:
+
+- `kvs set [KEY] [VALUE]`


I think KEY and VALUE are all required arguments. Is it better to change the square brackets to angle brackets <>?

Yes, good catch.

sticnarf · 2019-03-25T08:57:24Z

rust/projects/tools/project.md

+
+  Set the value of a string key to a string
+
+- `kvs get [KEY]`


06kellyjac · 2019-03-25T09:04:29Z

rust/.gitignore

@@ -1,2 +1,2 @@
 *~
-yarn-error.log
+target/


No newline at eof

Suggested change

target/

target/

sticnarf · 2019-03-25T10:18:59Z

rust/projects/tools/project.md

+
+  Print the version
+
+The `kvs` library contains a type, `KvStore`, that supports the following


The behavior is not clearly specified, for example, the behavior of getting the value of a non-existing key.

I'll change the return value of get to be Option<String>.

sticnarf · 2019-03-25T11:37:34Z

rust/projects/tools/project.md

 through the project, otherwise you will be distracted by the many failing tests
-that you are not currently working on fixing.
+that you have not yet fixed.

 **Question D**: even if a given test suite doesn't contain any tests, why
 might we not want them to run? Besides issuing the above command, how could


Do you mean "why might we want to to run?" here

No, I mean "not", but I can see how the sentence suggests the opposite. I'll change it to just "Why might we not want to run empty test suites". The only answer I'm thinking of has to do with compile times. This question may be too obscure to keep.

I still feel the question a bit confusing. Can we add something to lead readers to thinking about the compile time? For example, add "It takes no time to run an empty test.` to the beginning?

overvenus · 2019-03-25T14:30:57Z

rust/README.md

- Asyncronous programming with [futures] and [tokio]
- High-performance networking with [GRPC] and [prost]
+- Asyncronous programming with [futures]
+- Networking with [hyper]


Hyper focuses on the HTTP protocol. I wonder what kind of service this subject will accomplish? (Plain HTTP 1.0 APIs or HTTP2 steaming APIs)

I wasn't planning to do anything HTTP2 specific, though I assume hyper will negotiate a HTTP2 upgrade if it can. Do you have any suggestions?

I'm torn about using HTTP for this, since gRPC is what PingCAP uses. But @siddontang and I discussed that it may be too much to require gRPC for this project. If we don't do gRPC in the initial set of projects, I'm thinking it could be in a subsequent project.

I think teaching 'networking with Hyper' is not a good idea since we don't use it at pingCAP (afaik) and it is not an essential library and the domain is still in flux. I think it is OK to use Hyper, just emphasize that we're teaching networking in Rust rather than any particular library (could even just use TCP?)

@nrc Somewhere @sticnarf suggested having the student just implement basic HTTP for that project, which does force them to write "real" networking code (vs just dropping in hyper).

overvenus · 2019-03-25T14:50:48Z

rust/notes.md

@@ -37,6 +37,9 @@ in the README.
  - Ana likes this
 - variable shadowing
 - DSTs
+- configuring clippy / rustfmt
+- scripting clippy / rustfmt for CI
+- CI setup


+1, it is very important for open-source projects, I just fixed a bug today during setting up CI. But which one do we want to target? Maybe we can make a list of CI doc link, let students choose.

Offhand I'd prefer to be opinionated and pick either travis or circle, whichever has the most open source mindshare (or maybe whichever pingcap uses). That way we can be specific about requirements, and have a better chance at automatically verifying they set up CI correctly.

It's not a strong opinion though.

It's also a chance to talk about the README and other boilerplate files, the various badges and other services Rust projects typically use.

overvenus · 2019-03-25T14:55:17Z

rust/projects/tools/tests/tests.rs

@@ -0,0 +1,8 @@
+#[test]
+fn cli_no_args() {}


Shouldn't it be filled with a test case? So students can focus on implementation.

I will fill the tests.

overvenus · 2019-03-25T14:59:20Z

rust/projects/tools/src/lib.rs

@@ -0,0 +1,3 @@
+pub fn main() {


I think the main function name is confusing. It is the same name as binary entry points, but in this case, it is not.

Good feedback. I'll change it to run, but let me know if you've got a better name. Or better ideas about how to test the cli (I'd rather stay within cargo test than explain how some custom scripts work).

brson · 2019-03-26T03:32:48Z

Thanks so much for the reviews @sticnarf, @06kellyjac, @overvenus ! I've pushed a commit that addresses all of the feedback except for the problem that there's no synchronous hyper.

nrc

Mostly minor comments inline. Bigger picture, I think the questions would make good exam questions, but are perhaps not so good as exercises in lessons since there is usually just one right answer, and finding that answer probably won't help the student learn too much. For example,

Notice that there are four different set of tests running
(each could be called a "test suite"). Where do each of those test suites come
from?

This is a good check of knowledge, but the if the student is just googling this, then it feels a bit like trivia, and doesn't help them remember the knowledge. If you wanted to have a question like this, I think it would be better to direct them to ways to solve it, e.g., instead of asking "Where do each of those test suites come from?", ask "Can you write a test which is part of each of the four test suites?", then give a hint such as listing the four kind of tests with names which can be googled to lead to good descriptions.

My other big picture thing is that I worry about the overall goals of the course and how focussed it should be: is this an intermediate course which happens to use networking as a teaching tool? Or is it a networking course? Or a Rust networking course? etc. I think it would be good to think specifically about what audience we expect, and what 'problem' we are solving for them.

nrc · 2019-03-27T05:10:29Z

rust/README.md

- Asyncronous programming with [futures] and [tokio]
- High-performance networking with [GRPC] and [prost]
+- Asyncronous programming with [futures]
+- Networking with [hyper]


I think teaching 'networking with Hyper' is not a good idea since we don't use it at pingCAP (afaik) and it is not an essential library and the domain is still in flux. I think it is OK to use Hyper, just emphasize that we're teaching networking in Rust rather than any particular library (could even just use TCP?)

nrc · 2019-03-27T05:29:07Z

rust/plan.md

+counts toward extra credit during final evaluation_. Pull requests to any other
+project used during this course count as well. This is an opportunity to gain
+experience contributing to an open-source Rust project. Make this a better
+course for the next to take it than it was for you. That is how open source


Suggested change

course for the next to take it than it was for you. That is how open source

course for the next students to take than it was for you. That is how open source

nrc · 2019-03-27T05:30:55Z

rust/plan.md

@@ -24,209 +24,240 @@ A suggested workflow:
  - if taking the course online, read the writeups that accompany the slides
 - Follow each project according to their own instructions, writing Rust programs
  that pass the projects' accompanying tests.
- (Optionally) Submit project for online grading via [TODO].
+


I think we should have some explicit prerequisites - do we expect people to know Rust already? Or another programming language? Enough about DBs to know what a KV store is?

nrc · 2019-03-27T05:35:47Z

rust/plan.md

- Lesson: Formatting, println et. al, log, and slog
+**Topics**: `#[test]`, how does test work?, what does `cargo run` actually do?,
+clippy, rustfmt, controlling clippy and rustfmt, links to other useful tools,
+cargo / rustc wrapping pattern in depth (ex rustup, `RUSTC_WRAPPER`)


this doesn't feel like the other topics - it is probably worth thinking explicitly about how much internals stuff we want to cover (my vote would be for a minimal amount - just enough to understand things like repr(packed))

nrc · 2019-03-27T05:37:43Z

rust/plan.md

-  - Topics: formatting tips, derive Debug in depth, how does format! work?, log
-    demo, slog and structured logging, env_logger, APIs that take format
-    strings,
+### [Lesson: Formatting, println et. al, log, and slog][t-fmt] ([slides][s-fmt])


What is the motivation for this? I expect if the student already knows Rust basics, then they can use println and I don't think advanced format string stuff helps too much with the goal of networking

The motivation is to teach interesting intermediate Rust material that is related to the project at hand. This project has evolved such that there's not a lot of formatting going on though.

I've heard quite a bit of feedback about the scope of the project and the lessons not focusing on only the information needed for the projects.

I'm just going to delete all the lessons and focus on the projects for now.

nrc · 2019-03-27T05:53:01Z

rust/projects/tools/project.md

+things to do before it is a polished piece of Rust software, ready for
+contributions or publication.
+
+First, public items should have doc comments.


I think this is a bit strong, but this is kind of a bee in my personal bonnet

I'll add a qualifier.

nrc · 2019-03-27T05:53:22Z

rust/projects/tools/project.md

+the `target/doc` folder. Note though that `target/doc` folder does not contain
+an `index.html`. Your crate's documentation will be located at
+`target/doc/kvs/index.html`. You can launch a web browser at that location with
+`cargo doc --open`.


worth mentioning rustup doc here too

I don't know. It doesn't help complete the project. The only way I can think to fit it in is as an aside, like "Relatedly, when you need to browse Rust's own documentation, you can run rustup doc."

nrc · 2019-03-27T05:53:53Z

rust/projects/tools/project.md

+information gathered from the type signature. They explain why and how one would
+use a function, what the return value is on both success and failure, error and
+panic conditions. The library you have written is very simple so the
+documentation can be simple as well.


👍 though I might go further and say that the simple docs aren't necessary at all.

nrc · 2019-03-27T05:55:19Z

rust/projects/tools/project.md

+--doc`.
+
+You may want to add `#![deny(missing_docs)]` to the top of `src/lib.rs`
+to enforce that all public items have doc comments.


I hate making this lint deny, but it might be a minority opinion

nrc · 2019-03-27T05:55:51Z

rust/projects/tools/project.md

+
+```
+rustup component add clippy
+rustup component add rustfix


Suggested change

rustup component add rustfix

rustup component add rustfmt

Good catch.

brson · 2019-03-27T20:24:29Z

I think the questions would make good exam questions, but are perhaps not so good as exercises in lessons since there is usually just one right answer, and finding that answer probably won't help the student learn too much.

When I was writing the project, any time I hit a task that could be done multiple ways, with different pros and cons; or when I had some thoughts about the subject beyond the task at hand, I wrote it down as a question, with the intent that the student thinks deeper about the subject and also gains that knowledge.

Some of @sticnarf's comments on his own followup PR probably make two of the questions moot, so I'll probably just drop the notion of questions altogether.

brson · 2019-03-27T20:47:42Z

My other big picture thing is that I worry about the overall goals of the course and how focussed it should be: is this an intermediate course which happens to use networking as a teaching tool? Or is it a networking course? Or a Rust networking course? etc. I think it would be good to think specifically about what audience we expect, and what 'problem' we are solving for them.

Hi @nrc.

This question of the goals of the course, and the intended audience are distressing. It's been asked a number of times, and I've tried to convey it clearly in the readme.

Right now the readme contains this statement:

Goals of this course are to convey knowledge relevant to real-world Rust programming, including inherited wisdom and best practices; to look "under the hood" and understand how and why Rust works like it does; to tour the world of Rust documentation and code such that the student can learn how to find answers to their questions on their own.

And this

This book is not for novice programmers, but it is for novice Rust programmers.

This course is for programmers with some knowledge of data structures, algorithms and I/O.

This course is for programmers who have begun learning Rust, but still need guidance in production Rust programming and best practices.

This is a course for active learners — it will lead you in the right direction, but the journey is yours.

Those who complete the course will have the knowledge and experience to begin to write high performance, reliable, networked applications in Rust.

This course, combined with Deep Dive TiKV is intended to be enough to enable programmers to meaningfully contribute to TiKV or any other Rust project. It is most specifically designed to teach those in the Chinese Rust community enough Rust to work on TiKV. The language used is intended to be simple so that those who read only a little English can follow.

The goal statement expresses my teaching interests - I want to convey knowledge that can't be found in the books, the types of things that I find myself considering when writing Rust software. The projects - to me - are simply a backdrop for stirring up subjects to dig deep into.

This doesn't seem to resonate with others though, since I hear lots of confusion about the lesson subjects not being relevant to the projects.

So it looks like I need to adjust my idea for this course, focus on projects that teach the technologies that TiKV uses, providing the background material needed for that, and cut everything else out.

I'll think of new statements about the goals and audience, but I'm very glad to take suggestions. Maybe once we have a revised goal statement I or someone else will annotate every topic to explain the motivation, since that is a question that comes up on every pull request.

cc @sticnarf this thread has discussion about the goals of the project.

I've pushed a commit to address @nrc's suggestions, except for mentioning rustup doc, which I'm not convinced about. The larger questions of scope can be addressed later.

For the sake of progress I'm going to merge this, but please file bugs if you disagree with any of it.

nrc · 2019-03-28T11:22:28Z

I'll keep discussion of general goals on #24

This question of the goals of the course, and the intended audience are distressing.

Sorry, I don't mean to cause distress. A contributing factor may be that I have heard several different motivations for the course from different people at different times and I am taking that into account as well as the actual text.

For the sake of progress I'm going to merge this, but please file bugs if you disagree with any of it.

👍

brson added 7 commits March 21, 2019 10:39

Fix js for directory urls

b11be93

Make text a bit more readable w/ more padding

578f06d

Add cargo project for project 1

7f4fb0d

Make code more readable

207b056

Cleanup

8a49b1d

Some more project 1 text

7693500

Change the plan to not include grpc or tokio

296f0b2

Fill out project 1, etc.

32311e6

brson force-pushed the master branch from b756d89 to 32311e6 Compare March 25, 2019 01:59

sticnarf reviewed Mar 25, 2019

View reviewed changes

rust/projects/tools/project.md Outdated

Set the value of a string key to a string

- `kvs get [KEY]`

Copy link

Collaborator

sticnarf Mar 25, 2019

Choose a reason for hiding this comment

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

same

06kellyjac reviewed Mar 25, 2019

View reviewed changes

rust/.gitignore Outdated

@@ -1,2 +1,2 @@

*~

yarn-error.log

target/

Copy link

06kellyjac Mar 25, 2019

Choose a reason for hiding this comment

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

No newline at eof

Suggested change

target/

target/

sticnarf reviewed Mar 25, 2019

View reviewed changes

sticnarf mentioned this pull request Mar 25, 2019

Add tests and an example solution to Rust training Project 1 #10

Merged

sticnarf reviewed Mar 25, 2019

View reviewed changes

overvenus reviewed Mar 25, 2019

View reviewed changes

brson added 2 commits March 25, 2019 20:30

Rework from feedback

2468e86

Merge branch 'master' into master

1bfb08b

brson mentioned this pull request Mar 26, 2019

Write the project description for project 1 #2

Open

nrc reviewed Mar 27, 2019

View reviewed changes

sticnarf mentioned this pull request Mar 27, 2019

Redesign Rust networking progression #15

Closed

More feedback rework

f4afd81

brson force-pushed the master branch from 65ca6c4 to f4afd81 Compare March 27, 2019 20:29

brson merged commit 34e8eb0 into pingcap:master Mar 27, 2019

nrc mentioned this pull request Mar 28, 2019

Reformulate the goals and audience for this course #24

Open

	Proect text may include inline links to pages that offer explanatios of terms
	Project text may include inline links to pages that offer explanations of terms

		@@ -0,0 +1,89 @@
		TODO organize this

		Every md file has a html relative symlink to index.html:


		## Contributor notes

		- each md file needs a relative symlink to index.html


		- Lesson: Build-time Rust
		- Use hyper for synchronous networking

	Question D: even if a given test suite doesn't contain any tests, why
	Question D: Even if a given test suite doesn't contain any tests, why


		Again, the interface for the CLI is:

		- `kvs set [KEY] [VALUE]`


		Print the version

		The `kvs` library contains a type, `KvStore`, that supports the following

	course for the next to take it than it was for you. That is how open source
	course for the next students to take than it was for you. That is how open source

More rust training content #7

More rust training content #7

Conversation

brson commented Mar 23, 2019

brson commented Mar 23, 2019

brson commented Mar 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brson commented Mar 26, 2019

nrc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brson commented Mar 27, 2019

brson commented Mar 27, 2019

nrc commented Mar 28, 2019