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

reintroduce setup from inputstream #15

Merged
merged 4 commits into from
Jun 7, 2019
Merged

Conversation

marekpalenik
Copy link

No description provided.

@ktor
Copy link
Member

ktor commented May 21, 2019

Hi @marekpalenik, why do you need that?

@marekpalenik
Copy link
Author

marekpalenik commented May 21, 2019

@ktor : 1. customers' project setup uses inputstreams
2. my private testing/learning project uses inputstreams
3. JAXBException in MarshallUtil with: 'Please fix the library' for which i do not have time now.

@ktor
Copy link
Member

ktor commented May 21, 2019

@marekpalenik, thx, until this is resolved- version 2.1.4 still has InputStreams if you need to be fast with the upgrade. You can download it here: https://github.com/ableneo/liferay-db-setup-core/releases/tag/2.1.4

What you've written just states that you use the method, I understand that. Why do you need it? Could you use anything else than an InputStream?

2.2.x is incompatible with 2.1.x for different reasons but also because I hesitate to support InputStream in public api. Why? When an InputStream is managed outside of the library and not properly managed (closed) it can easily create leaks (steal limited system resources). I feel this is critical as we use the library in production setups. To reduce the risk I've chosen to open and close streams inside within the library to stay safe.

Would String method argument be OK in your setup?

@ktor
Copy link
Member

ktor commented May 23, 2019

Hi Marek,

I was thinking about it and discussing with colleagues. My arguments against InputStream seems weaker and weaker to me. Let's make a following deal.

LiferaySetup will only accept Setup object as an argument to work with and MarshallUtil will parse xml from whatever source, including InputStream.

We'll separate responsibility for unmarshalling away from setup operations.

What do you think?

@ktor ktor self-requested a review June 7, 2019 16:03
@ktor ktor self-assigned this Jun 7, 2019
@ktor ktor merged commit 7f25854 into ableneo:master Jun 7, 2019
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

Successfully merging this pull request may close these issues.

2 participants