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

The "any" type does not accept non-implemented types like timestamp #14

Closed
sitaktif opened this issue Mar 11, 2015 · 10 comments
Closed

Comments

@sitaktif
Copy link

The documentation specifies that any accepts any implemented type.

Is there a reason behind throwing a validation error when one uses other non-implementd types like timestamps?

I understand that a possible fix in my case is to send a PR to add timestamps support - and I'll do that if I get around to it :-)

Thanks,

@Grokzen
Copy link
Owner

Grokzen commented Mar 11, 2015

@sitaktif When you say timestamp do you mean a string or integer like "1426059442" || 1426059442 or do you mean that your object is a python datetime object? Is it possible that you can provide a sample schema file and data file so i can test the problem myself?

The reason behind throwing the validation error is simple and that is because i have to implement the check for a given type and that is what any is for. A improvment upon this problem might be to implement a lower type then any that basiclly do nothing and accepts anything perhaps?

@Grokzen
Copy link
Owner

Grokzen commented Mar 19, 2015

@sitaktif Your question answers itself i think because the docs state how it works and that will not change any time soon. A simple solution could be to use str or int depending on your data files and let it validate through that one. I will add a new type in the future that is something more in the line off all or whatever or something that validates no matter what it says. To add datetime and timestamp support is something in the pipe but like yourself i have not got around to do it :]

@Grokzen Grokzen closed this as completed Mar 19, 2015
@sitaktif
Copy link
Author

@Grokzen Thanks for your answer! And to clarify, by timestamp I meant something that looks like 2015-03-27T21:07:46Z. I believe this is what the vanilla Kwalify library calls it (I was myself confused by the naming).

The reason behind throwing the validation error is simple and that is because i have to implement the check for a given type and that is what any is for.

I am not sure I understand why you have to check for all the types that any checks for. Is it for security reasons?

The current meaning for any is a bit hard to handle at the moment because each time a new type gets supported, it is going to break backward compatibility in the API. Indeed, when the timestamp (or let's call that datetime rather) gets added, any's behaviour will suddenly change (it will accept values that it used to deny).

Also, accepting any type (even the non-implemented ones) with any will make it more compatible with the original Kwalify, and will allow people to use a given Kwalify schema in Python, Ruby and Java. Adding a new keyword such as all or whatever will unfortunately not fix that issue.

This is maybe not particularly relevant to the development of Pykwalify, but to give you one data point, the main reason I got interested in Pykwalify was that I could use the same Yaml schema in different languages. This is why compatibility between the different kwalify libraries is important to me -- but again I understand this might not be one of your goal maintaining this particular lib.

@Grokzen
Copy link
Owner

Grokzen commented Mar 29, 2015

The original documentation is lacking with the defenition of the major difference between, time, date and timestamp.

I would say that here in the python version it could be defined as

time - Format like HH-MM-SS it should be configurable what formats that should be supported ofc but the default list should contain all the most used formats.

date - Format like YYYY-MM-DD or MM-DD-YYYY Here also it should be configurable what formats should be supported.

timestamp - I define this as a unix timestamp that you derive from

>>> import time
>>> time.time()
1427595892.498187

It could be valid with or without decimal numbers but it could be configurable.

datetime - This should match against the normal datetime formats but should also be configurable what formats should be supported.

I understand your argumentation with the any rule and i could agree to change it to your defenition if it brings the schema closer to the original kwalify schema.

@sitaktif
Copy link
Author

The original documentation is lacking with the definition of the major difference between, time, date and timestamp.

I agree the Kwalify documentation is lacking on this but putting Kwalify (and Pykwalify) aside, we should follow standards and not come with our own recipes. Doing it in a Pythonic way is not necessarily the right way to go (and believe me, I love the pythonic way :-) ) because Yaml is a concept that is language agnostic.

You can see an example of timestamps in the Yaml official standard on [http://www.yaml.org/spec/1.2/spec.html#id2761573](see the "Example 2.22. Timestamps" section). It follows the ISO8601 format (see http://en.wikipedia.org/wiki/ISO_8601), which is one of the widely used standards for timestamps (or what we would call datetime).

Btw, the way I described above is the way Kwalify behaves. You can look at the code, but I built an example to be sure:

$ cat schema.yaml
type: timestamp
$ cat just_an_int.yaml
1427650980
$ cat full_datetime.yaml
2015-03-29T18:45:00+00:00
$ ./validate.rb schema.yaml just_an_int.yaml
[/] '1427650980': not a timestamp.
$ ./validate.rb schema.yaml full_datetime.yaml
2015-03-29 11:45:00 -0700

The code for validate.rb is 90% taken for the Kwalify documentation example:

#!/usr/bin/ruby

require 'kwalify'
require 'yaml'

schema=ARGV[0]
document=ARGV[1]

schema = YAML.load_file(schema)
validator = Kwalify::Validator.new(schema)
document = YAML.load_file(document)
errors = validator.validate(document)

if errors && !errors.empty?
  for e in errors
    puts "[#{e.path}] #{e.message}"
  end
else
  puts document
end

i could agree to change it to your defenition if it brings the schema closer to the original kwalify schema.

That's good to know. It's a backward incompatible change so it's probably better to postpone it to a sort of "major" release, but I think it's good to keep this change in mind.

Thanks!

@Grokzen
Copy link
Owner

Grokzen commented Mar 29, 2015

If i change the any i would tag it with a new major release 1.1.0 and just document a upgrade notice that the defenition has changed. There is nothing stopping me from doing this right now if i want so i probably will push it sometime during the week :]

Your exampleis very good and i can easily try to implement something similar to that.

@Grokzen Grokzen reopened this Apr 1, 2015
@Grokzen
Copy link
Owner

Grokzen commented Apr 1, 2015

@sitaktif Do you know some good list or site or something that could help to determine what formats to support out of the box? I am thinking about adding ability to manually specify what formats that you want for a datetime but there must be a default set also.

@Grokzen
Copy link
Owner

Grokzen commented Apr 4, 2015

Timestamp implemented in c5a4782 Will be included in next tag/release.

@Grokzen
Copy link
Owner

Grokzen commented Apr 4, 2015

The any type change is now implemented and new release 1.1.0 is released and uploaded.

If you have any comments or suggestions on the timestamp implementation please open a new issue :]

@Grokzen Grokzen closed this as completed Apr 4, 2015
@sitaktif
Copy link
Author

sitaktif commented Apr 4, 2015

@Grokzen Nice!

This is coming a bit late (sorry...) but I do have a suggestion about what formats to support: why don't we use yaml.dump(the_date_value) to support exactly what the actual yaml library supports? That way there is no question of whether we accept invalid formats (or whether we throw an error on a yaml-compliant format).

Using the yaml library to parse the date delegates this kind of decision to the people maintaining the yaml lib and ensures the formats are aligned.

EDIT: I'll open a new issue to discuss this.

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

No branches or pull requests

2 participants