You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "zhangwl9 (via GitHub)" <gi...@apache.org> on 2024/12/06 10:48:42 UTC

[I] [Improvement]: Add a commonly used MySQL dependency and activate a profile to include it [amoro]

zhangwl9 opened a new issue, #3352:
URL: https://github.com/apache/amoro/issues/3352

   ### Search before asking
   
   - [X] I have searched in the [issues](https://github.com/apache/amoro/issues?q=is%3Aissue) and found no similar issues.
   
   
   ### What would you like to be improved?
   
   Currently Amoro frequently uses MySQL as a metadata database, but does not provide an option to add MySQL dependencies in Amoro package. To solve the problem of  MySQL dependencies, we would like to add the MySQL dependency in Maven with a 'provided' scope, and then change it to 'compile' by activating a profile to include the dependency.
   
   
   
   ### How should we improve?
   
   After this solution takes effect, we can include the MySQL dependency by executing mvn clean package install -Dinclude-mysql=true 
   
   ### Are you willing to submit PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Subtasks
   
   _No response_
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2586016167

   The issue is that it seems like it is not an optional dependency, so no it is not in line with ASF policy, as the majority of users would use it. ASF products can not depend on software with a GPL license [1], so it is an issue just depending on it and not including it. Even if that was not the case, the user needs a lot more warning that what they are going to do is incompatible with the Apache license.
   
   1. https://www.apache.org/licenses/GPL-compatibility.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589101818

   Sure, maybe we should roll back #3380 and keep the original method of manually downloading the MySQL driver.
   
   HDYT? @klion26 @zhangwl9 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589137633

   Why would you point the users to software incompatible with the Apache license and put the project's license and compliance with ASF policy at risk? Sure, please document it, but it should not be prominent as otherwise, is not an optional dependency. Where it is documented, it must be made very clear that it's incompatible and the user uses at their own legal risk.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2588837041

   Thanks for pushing forward this issue. @klion26 And thanks for providing the license information. @justinmclean 
   
   I think there are some erros with the description of this issue like "Currently Amoro frequently uses MySQL as a metadata database". We have **NEVER** collected information about what database Amoro users used and come to the conclusion that most users use MySQL as Amoro's meta store.
   
   Users can use any database as Amoro's meta store as it provides a JDBC implementation and Amoro will not rely on the unique features of any specific database.
   
   We may give some guide for users who used MySQL as its meta store and profile is fine to me as if we should tell them the license incompatible issues in the profile description.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2590102736

   This is much more than a documentation issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2586009437

   @justinmclean Thanks for reminding us about the license information. Currently, Amoro stores the information used for ha/upgrade in a database(which can be MySql or Postgresql for now), and we do not include MySQL when releasing.
   
   This issue here wants to add a profile that can be activated(default value is off) by the user who uses Mysql as the database to store their data used by Amoro, the dependency of MySQL would not been included in the Apache Amoro distribution. In other words, this issue did not change the content for the Apache Amoro distribution, it only added a feature for the users to easily package the MySQL dependency into their inner distribution or in their development env.
   
   Does this align with the Apache Linces? or we can just add a `provided` dependency of MySql in our pom.xml, and can' support such profile to change the dependency scope from `provided` to `compile` in the pom.xml(with default value to `provided` and releasing with `provided` scope)
   
   cc @zhoujinsong please correct me if there is something wrong
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2584590763

   MySQL's license is not compatible with the Apache license, so you should not be this. Can you please expand on "Currently Amoro frequently uses MySQL as a metadata database," because if it is used by most people, then is it not an optional dependency. See https://www.apache.org/legal/resolved.html#optional


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "XBaith (via GitHub)" <gi...@apache.org>.
XBaith commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589428445

   > Sure, maybe we should roll back [#3380](https://github.com/apache/amoro/pull/3380) and keep the original method of manually downloading the MySQL driver.
   
   +1 for revert the PR ASAP
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "zhangwl9 (via GitHub)" <gi...@apache.org>.
zhangwl9 commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589127874

   > Sure, maybe we should roll back [#3380](https://github.com/apache/amoro/pull/3380) and keep the original method of manually downloading the MySQL driver.当然,也许我们应该回滚并保留手动下载 MySQL 驱动程序的原始方法。
   > 
   > HDYT?   HDYT?[@klion26](https://github.com/klion26) [@zhangwl9](https://github.com/zhangwl9)
   
   @justinmclean @zhoujinsong  Should or can we provide an address for MySQL dependencies on the main GitHub page or the Amoro website? This way, when users want to use MySQL as the metadata, they can know what dependencies need to be added on that page and quickly find out where to download them.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong closed issue #3352: [Improvement]: Add mysql dependency by maven profile
URL: https://github.com/apache/amoro/issues/3352


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589464907

   Looking at the issue a bit further, it seems it deeper than this one issue. I can see several places where it is stated that users should use MySQL in production.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2588852165

   If most users rely on MySQL as Amoro's meta store, then it can't be considered an optional dependency. However, if MySQL usage is uncommon, there wouldn't be a need for this issue or PR. This creates a contradiction: either MySQL is widely used or it isn’t. Which is the case?
   
   Making installation easier for users also places greater responsibility on the project to ensure they fully understand the implications of their choice. In my opinion, simply documenting the license is insufficient. For example, you could display a clear warning during installation, explicitly stating that it is not compatible. Users should then confirm their understanding by responding Y/N to a prompt before proceeding with the installation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2587023634

   I feel you are misisng the main point of this conversation. What was stated above implies this dependency is not truly optional; is this the case or not? If this is not the case, how are users informed that MySQL is incompatible with the Apache license?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589013115

   No, I do not, as I explained above.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2586942252

   @justinmclean Currently, there are some docs [1] for the users who use MySQL as the database(show that they need to download the MySQL dependency.  (users can choose the database from Derby/PostgreSQL/MySQL)
   
   > You can use MySQL/PostgreSQL as the system database instead of the default Derby.
   > If you would like to use MySQL as the system database, you need to manually download the [MySQL JDBC Connector](https://repo1.maven.org/maven2/com/mysql/mysql-connector-j/8.1.0/mysql-connector-j-8.1.0.jar) and move it into the {AMORO_HOME}/lib/ directory. You can use the following command to complete these operations:
   
   I checked the Flink JDBC connector before reviewing this change, it is released without the MySQL dependency, and the doc[2] says that users need to download the dependency manually.
   
   > The JDBC connector is not part of the binary distribution. See how to link with it for cluster execution [here](https://nightlies.apache.org/flink/flink-docs-release-1.20/docs/dev/configuration/overview/).
   > A driver dependency is also required to connect to a specified database. Here are drivers currently supported:
   
   Are there any best practices for Amoro use cases for MySQL dependency here, thanks.  
   
   [1] https://amoro.apache.org/docs/latest/deployment/#configure-system-database
   [2] https://nightlies.apache.org/flink/flink-docs-release-1.20/docs/connectors/table/jdbc/#dependencies


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589518165

   filed an issue(#3404) to track the MySQL usage documentation problem.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2588892377

   Yes, having the users only able to install the MySQL drivers manually is what exactly our choice for now.
   
   The difference is that before this PR, users needed to manually download this dependency and place it in the project, while after this PR, users needed to manually enable the profile and rebuild the project.
   
   Do you think this change is okay? @justinmclean 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "justinmclean (via GitHub)" <gi...@apache.org>.
justinmclean commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2588824029

   Just because a user can choose doesn't mean it is optional in teh ASF sense. Please answer the questions asked above.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2588808117

   @justinmclean The MySQL dependency for Amoro is optional. Amoro would not be dependent on any MySQL-specific function; it only depends on common database connections and SQL grammar. Users can choose any database they are familiar with, MySQL is just one of the choices.
   
   We'll use this issue to add some more information about the MySQL dependency license to the users
   - move the whole dependency of MySQL to the optional profile
   - add license information about MySQL in the **Amoro docs/pom profile/[build guild](https://github.com/apache/amoro?tab=readme-ov-file#building)** to inform users that MySQL license is not compatible with the Apache License
   
   what do you think about this, thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2589115832

   @justinmclean @zhoujinsong Thanks for the detailed reply. I'm ok with the rollback.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong closed issue #3352: [Improvement]: Add mysql dependency by maven profile
URL: https://github.com/apache/amoro/issues/3352


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Improvement]: Add mysql dependency by maven profile [amoro]

Posted by "zhangwl9 (via GitHub)" <gi...@apache.org>.
zhangwl9 commented on issue #3352:
URL: https://github.com/apache/amoro/issues/3352#issuecomment-2561748859

   @klion26 @czy006 Could you please take a look at this when you're free, thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org