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

Documentation is incorrect/confusing for Timestamp.parseTimestamp #4583

Closed
dclements opened this issue Feb 27, 2019 · 2 comments
Closed

Documentation is incorrect/confusing for Timestamp.parseTimestamp #4583

dclements opened this issue Feb 27, 2019 · 2 comments
Assignees
Labels
api: core priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@dclements
Copy link

The code currently in master is:

/**
   * Creates a Timestamp instance from the given string. String is in the RFC 3339 format without
   * the timezone offset (always ends in "Z").
   */
  public static Timestamp parseTimestamp(String timestamp) {
    Instant instant = Instant.parse(timestamp);
    return ofTimeSecondsAndNanos(instant.getEpochSecond(), instant.getNano());
  }

But if we look at the code for org.threeten.bp.Instant.parse:

/**
     * Obtains an instance of {@code Instant} from a text string such as
     * {@code 2007-12-03T10:15:30.000Z}.
     * <p>
     * The string must represent a valid instant in UTC and is parsed using
     * {@link DateTimeFormatter#ISO_INSTANT}.
     *
     * @param text  the text to parse, not null
     * @return the parsed instant, not null
     * @throws DateTimeParseException if the text cannot be parsed
     */
    public static Instant parse(final CharSequence text) {
        return DateTimeFormatter.ISO_INSTANT.parse(text, Instant.FROM);
    }

This implies to me that per the Google Cloud Timestamp documentation the following should work:

Timestamp.parseTimestamp("2019-01-23T12:30:00")

But it generates:

org.threeten.bp.format.DateTimeParseException: Text '2019-01-23T12:30:00' could not be parsed at index 19

It also would imply to me that the following shouldn't work:

Timestamp.parseTimestamp("2019-01-23T12:30:00Z")

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Feb 28, 2019
@sduskis sduskis added type: docs Improvement to the documentation for an API. and removed triage me I really want to be triaged. labels Feb 28, 2019
@sduskis
Copy link
Contributor

sduskis commented Feb 28, 2019

@pmakani, can you please take a look at this?

@pmakani
Copy link

pmakani commented Feb 28, 2019

@sduskis working on this.

@pmakani pmakani added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: core and removed type: docs Improvement to the documentation for an API. labels Mar 7, 2019
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Mar 7, 2019
@pmakani pmakani self-assigned this Mar 7, 2019
@pmakani pmakani removed triage me I really want to be triaged. 🚨 This issue needs some love. labels Mar 7, 2019
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Mar 7, 2019
@sduskis sduskis added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: core priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants