SweetXML as hard dependency on 0.4.10?

[jordi-chacon]

Hi Ben,

just updated ex_aws dependency on my project from 0.4.8 to 0.4.10 and now get this error on compilation:

== Compilation error on file lib/ex_aws/s3/parsers.ex ==
** (CompileError) lib/ex_aws/s3/parsers.ex:3: module SweetXml is not loaded and could not be found
    (elixir) expanding macro: Kernel.if/2
    lib/ex_aws/s3/parsers.ex:2: ExAws.S3.Parsers (module)
    (elixir) lib/kernel/parallel_compiler.ex:97: anonymous fn/4 in Kernel.ParallelCompiler.spawn_compilers/8

could not compile dependency ex_aws, mix compile failed. You can recompile this dependency with `mix deps.compile ex_aws` or update it with `mix deps.update ex_aws

Has SweetXML become a mandatory dependency?

Thanks!

[benwilson512]

It's still listed as an optional dependency, let me see what's going on.

[benwilson512]

Not entirely sure what's going on there, the line that's erroring for you is inside this if statement https://github.com/CargoSense/ex_aws/blob/master/lib/ex_aws/s3/parsers.ex#L2 which ought to prevent this kind of thing. Will look into it more.

[ericmj]

@benwilson512 imports are lexical, meaning that they are always used regardless of which code path is executed. If you put the if outside of the defmodule it will work, because the module won't even be compiled.

Is there a reason for generating a module with stub functions if SweetXml is not used?

[benwilson512]

@ericmj so this is weird. Prior to the last release, I had sweet_xml listed as a dev dependency but that was it. While it was merely a development dependency it was entirely possible to have ex_aws as a dep without also having sweet_xml, and would compile. However, after adding it as an optional dependency we now have the above error.

What's the best way to achieve what I want here? The use case here is that streaming functions require that you parse the XML so it can handle pagination and build the list of items. However, such functionality isn't necessary if all you're doing is basic get/put object so I want to make it optional.

[ericmj]

The previous configuration of having sweet_xml as a dev dependency was incorrect. Optional and dev work differently so there is no surprise it no longer works.

I gave a suggestion for how to fix it in my previous comment:

If you put the if outside of the defmodule it will work, because the module won't even be compiled.

But to be able to help out more I need to know some information:

Is there a reason for generating a module with stub functions if SweetXml is not used?

[benwilson512]

In answer to your latter question: At the moment I'm doing https://github.com/CargoSense/ex_aws/blob/master/lib/ex_aws/s3/impl.ex#L53 where if you have sweet_xml installed it'll get parsed, otherwise it won't. On reflection, this probably isn't the right call as it's implicit.

The streaming code should call it explicitly, and the basic impl call should leave it alone. The upcoming changes will make a nicer line between what should parse by default and what shouldn't.

[benwilson512]

The easiest non breaking change it seems however would just be to have two versions of the same module, one inside each branch of the if,

[ericmj]

If I understand it correctly you will get a pretty obscure error message if SweetXml is not present. Why even generate the stub module?

[benwilson512]

Right now every call to list_objects calls Parsers.parse_list_objects. If SweetXML is present, it'll get parsed, if not, it'll just pass the value along. I didn't want to check for the presence of SweetXML at runtime as that would be a lot of avoidable overhead.

[ericmj]

Won't it error if SweetXml is not there anyway? Or is it valid to not parse the response? It feels like you are just going around "fail fast".

EDIT: You don't have to check for the presence, just let the runtime raise the normal error.

[benwilson512]

It's valid to not parse the response until I've built parsers for every request such that it consistently returns a parsed value. This is contingent upon generating parsers, a step that is in progress.