[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[tliron]

Github user tliron commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
OK. So I guess you don't want to change the imports in `/tests/`?

On Wed, Nov 8, 2017 at 4:32 PM, Maxim Orlov 
wrote:

> so, i think we're ready
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the thread
> 

> .
>



---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[mxmrlv]

Github user mxmrlv commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
so, i think we're ready


---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[tliron]

Github user tliron commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
I think we are. Also: I only suggest, you of course can decide differently.
Maybe do one more commit to make sure we are on the same page? Also feel
free to rebase and squash, it's actually easier for a small PR like this.

On Wed, Nov 8, 2017 at 2:44 PM, Maxim Orlov 
wrote:

> OK, np just wanted to make sure we're on the same page...
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the thread
> 

> .
>



---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[mxmrlv]

Github user mxmrlv commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
OK, np just wanted to make sure we're on the same page...


---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[tliron]

Github user tliron commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
Still, I don't see why it should be in `aria/__init__.py` and be always
activated. I think it should be in activated only for a module that
specifically is using ruamel. It would still only happen once.

On Wed, Nov 8, 2017 at 2:31 PM, Maxim Orlov 
wrote:

> This code only runs once, it doesn't provide with a patched up ruamel, it
> patched the loading mechanism...so loading should work everywhere, as long
> as the code was run...
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the thread
> 

> .
>



---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[mxmrlv]

Github user mxmrlv commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
This code only runs once, it doesn't provide with a patched up ruamel, it 
patched the loading mechanism...so loading should work everywhere, as long as 
the code was run...


---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[tliron]

Github user tliron commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
@mxmrlv You didn't address these:

1. Let's move this to `from aria.utils.yaml import yaml`. I don't think the 
aria `__init__.py` is the right place for this.
2. Let's go over *all* places in the codebase where we import ruamel to use 
the above.


---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[mxmrlv]

Github user mxmrlv commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
@tliron any additional comments? or should i merge?


---

[GitHub] incubator-ariatosca issue #199: ARIA-392 Failing to load ruamel.yaml

[asfgit]

Github user asfgit commented on the issue:

https://github.com/apache/incubator-ariatosca/pull/199
  
Can one of the admins verify this patch?


---