Allow pre loaded AsyncAPISpec in from_spec

[alex-zywicki]

Allow from_spec to accept an AsyncApiSpec object or the Path to an asyncapi file.
This will aid in the implementation of spec merging from #108 by allowing the programmer to load and merge specs and then pass in the merged spec rather than having to write the merged spec back out to file.

[codecov-commenter]

Codecov Report

Merging #160 (cacffc9) into main (5466483) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #160      +/-   ##
==========================================
+ Coverage   97.09%   97.11%   +0.01%     
==========================================
  Files          11       11              
  Lines         895      900       +5     
==========================================
+ Hits          869      874       +5     
  Misses         26       26              

Flag	Coverage Δ
unittests	97.11% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
asynction/mock_server.py	93.44% <100.00%> (+0.05%)	⬆️
asynction/server.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5466483...cacffc9. Read the comment docs.

[dedoussis]

Suggested change

	spec: Union[Path, AsyncApiSpec],
	spec_path: Union[Path, JSONMapping],

2 points to discuss here:

1. I would be keen to avoid exposing the AsyncApiSpec type as part of Asynction's public API. This will restrict us from being able to make backwards incompatible changes to the class, unless we bump the major version of the whole package. Besides the type, we will also have to expose the load_spec method (the type alone is pretty useless), which I would also like to avoid for the same reason. Instead of passing the typed object, we could have users passing the raw dict of their AsyncAPI spec. Hence, we would have the from_spec factory method supporting file paths as well as raw dict data. The latter will account for use cases such as deep merging multiple spec files into one dict (the use case of yours) or converting class definitions to JSONSchemata. When it comes to the merging of multiple files, there is the shortcoming that without the type and the loader function users won't be able to ensure that each of the spec files has a valid and complete AsyncAPI data structure. However I would argue that this is not the responsibility of Asynction. This would be the responsibility of a parser library like asyncapi/parser-js. Unfortunately, python does not have such a library yet, but I'm currently working on one that I expect to release towards the end of Jan.
2. Renaming the argument name from spec_path to spec is a backwards incompatible change to Asynction's public API, which will require a major version bump. Of course spec_path is not an accurate name anymore, but I would like to introduce this change only when Asynction is ready for the 1.0.0 bump. I will create an issue where we can collect all the items that we need to modify ahead of the 1.0.0 stable release.

[dedoussis]

When it comes to the merging of multiple files, there is the shortcoming that without the type and the loader function users won't be able to ensure that each of the spec files has a valid and complete AsyncAPI data structure. However I would argue that this is not the responsibility of Asynction.

@alex-zywicki For the time being, If there is a requirement on your end to ensure that each file has a complete and valid AsyncAPI structure, you can use the existing methods of Asynction, but with the risk of having to modify parts of your code when you upgrade to a new minor version of the package.

[alex-zywicki]

@dedoussis It is not an issue on my end to have to modify my code if something in the types.py changes. I have my asynction version pinned to nothing will break unless I explicitly choose to upgrade the version. I will amend the pr to undo the name changes.

[dedoussis]

Suggested change

	spec_path: Union[Path, AsyncApiSpec],
	spec_path: Union[Path, JSONMapping],

See the previous comment I made with regards to exposing the AsyncApiSpec type.

[alex-zywicki]

@dedoussis Are you saying to just change the type annotations? how would the implementation change? Would I still be able to pass an AsyncApiSpec or what would I pass?

[dedoussis]

@alex-zywicki This would also be an implementation change.

I propose the from_spec method to accept either a file path or a dict. In the case of dict, the conversion of the given dictionary into an AsyncApiSpec object should happen internally within Asynction. This way we can avoid exposing the type to the user. See point 1 of my comment above:

Instead of passing the typed object, we could have users passing the raw dict of their AsyncAPI spec. Hence, we would have the from_spec factory method supporting file paths as well as raw dict data. The latter will account for use cases such as deep merging multiple spec files into one dict (the use case of yours) or converting class definitions to JSONSchemata.

[alex-zywicki]

@dedoussis I guess I'm confused about what you meant when you said a while back

@alex-zywicki For the time being, If there is a requirement on your end to ensure that each file has a complete and valid AsyncAPI structure, you can use the existing methods of Asynction, but with the risk of having to modify parts of your code when you upgrade to a new minor version of the package.

[dedoussis]

Looks sensible, thank you! Couple of minor comments.

[dedoussis]

Suggested change

	:param spec_path: The path where the AsyncAPI YAML specification is located,
	or a pre loaded JSONMapping object.
	:param spec_path: The path where the AsyncAPI YAML specification is located,
	or a dictionary object of the AsyncAPI data structure.

[dedoussis]

I think it would be neater if we could move this logic within the existing load_spec function:

def load_spec(raw_spec: Union[Path, JSONMapping]) -> AsyncApiSpec:
    if isinstance(spec, Path): 
        with open(spec_path) as f:
            serialized = f.read()
            raw_spec = yaml.safe_load(serialized)

    raw_resolved = resolve_references(raw_spec)
    return AsyncApiSpec.from_dict(raw_resolved)

Note that here, in the load_spec method, we can change the argument name from spec_path to raw_spec since load_spec is not part of Asynction's "public" API.

[dedoussis]

The above line in the from_spec method would then become:

Suggested change

	if isinstance(spec_path, Path):
	spec = load_spec(spec_path=spec_path)
	else:
	raw_resolved = resolve_references(raw_spec=spec_path)
	spec = AsyncApiSpec.from_dict(raw_resolved)
	spec = load_spec(raw_spec=spec_path)