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

Append-friendlier serial tiles serializer #206

Merged
merged 1 commit into from
Dec 29, 2017
Merged

Append-friendlier serial tiles serializer #206

merged 1 commit into from
Dec 29, 2017

Conversation

yhahn
Copy link
Member

@yhahn yhahn commented Dec 28, 2017

Context

The serialtiles serializer is written like a pretty reasonable newline delimited stream writer with the one exception that in the final _flush case it decides to not write a trailing newline. This causes all kinds of headaches for anyone attempting to resume or append to a previously written serialtiles files.

What happens now (newlines visualized for ease). Note that the last line does not have a trailing \n.

JSONBREAKFASTTIME\n
{ ... }\n
{ ... }\n
{ ... }

Now let's append to this stream assuming we can write in a way consistent with the previous writes...

# appended version
JSONBREAKFASTTIME\n
{ ... }\n
{ ... }\n
{ ... }{ ... }
{ ... }\n
{ ... }\n
...

Not great.

So then appenders do some trickery...

No big deal, appenders can just write lines with a prefixed newline! \n{ ... }

# appended version
JSONBREAKFASTTIME\n
{ ... }\n
{ ... }\n
{ ... }
\n{ ... }
\n{ ... }
\n{ ... }

Great! But what if our append starts after the initial header is written or while another initial stream is writing rather than after the first stream has concluded?

# appended version
JSONBREAKFASTTIME\n
\n{ ... }
\n{ ... }
\n{ ... }

Oww, now we end up with an empty line (the empty string sandwiched between \n\n). 🍔


Proposed solution

  • Always write a trailing newline. No weird exception on flush.
  • Add no-op handling for an empty string in deserialize. This is for the empty string sandwiched between \n\n for anyone who's been trying to append and has hit this corner case.
  • This branch also adds error handling for bad data passed for serialization (refs tilelive.serialize() should throw on invalid Tile obj #117).

This is a breaking change though some testing indicates previous versions of tilelive handle a single trailing \n on serialtiles files fine.

For safety will bump major.

cc @rclark @mapbox/core-tech

…ing newlines to make serialtiles appending more palatable.
@yhahn yhahn merged commit b926991 into master Dec 29, 2017
@yhahn yhahn deleted the append-friendly branch December 29, 2017 08:33
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.

1 participant