[GitHub] jena issue #189: JENA-1258 NodeFmtLib calls JenaSystem initializer

[stain]

Github user stain commented on the issue:

https://github.com/apache/jena/pull/189
  
Yes, your pull request looks like the best approach.

On 8 Nov 2016 12:33 pm, "Andy Seaborne"  wrote:

> The better fix is to initialize in NodeFactory (and Node for legacy
> reasons) only. I have tested this in a build.
>
> There is little point in a specific test because tests run in the overall
> suite; it's almost confusing to have a test that doesn't. With
> initialization in NodeFactory this will get tested anyway!
>
> Also, it would help to unroll ARQConstants (for RDF, RDFS, OWL, XSD) so it
> does not need JenaSystem.init and then it is, with care, init-safe.
>
> See PR#192 for that all made real.
>
> The original example works when run separately to ensure minimal system
> initialization.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> , or mute
> the thread
> 

> .
>



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #189: JENA-1258 NodeFmtLib calls JenaSystem initializer

[afs]

Github user afs commented on the issue:

https://github.com/apache/jena/pull/189
  
The better fix is to initialize in `NodeFactory` (and `Node` for legacy 
reasons) only. I have tested this in a build.

There is little point in a specific test because tests run in the overall 
suite; it's almost confusing to have a test that doesn't. With initialization 
in `NodeFactory` this will get tested anyway!

Also, it would help to unroll ARQConstants (for RDF, RDFS, OWL, XSD) so it 
does not need JenaSystem.init and then it is, with care, init-safe.

See PR#192 for that all made real.

The original example works when run separately to ensure minimal system 
initialization.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #189: JENA-1258 NodeFmtLib calls JenaSystem initializer

[afs]

Github user afs commented on the issue:

https://github.com/apache/jena/pull/189
  
The root cause:

Commit 204f253 switched Context to using a `ConcurrentHashMap`, commit 
d67a426 added a check for null otherwise nothing has changed since 3.1.0.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #189: JENA-1258 NodeFmtLib calls JenaSystem initializer

[stain]

Github user stain commented on the issue:

https://github.com/apache/jena/pull/189
  
Except for `impl` packages and suffixes it is never easy for Jena users to 
know what is "internal code". 

If it's in 
[Javadoc](https://jena.apache.org/documentation/javadoc/arq/org/apache/jena/riot/out/NodeFmtLib.html),
 and it is useful "outside" without much hazzle, then I don't see why it should 
be called "internal" just because it is also used internally.

Agree that it would be good to ensure Jena initialization from 
`NodeFactory`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jena issue #189: JENA-1258 NodeFmtLib calls JenaSystem initializer

[afs]

Github user afs commented on the issue:

https://github.com/apache/jena/pull/189
  
This usage is calling into the middle of internal code.

Fixing output code seems wrong. It is better to initialize as objects get 
created, not on output. This shows the root cause is somewhere else.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---