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

Add support for using aggrid in enterprise mode #3757

Closed

Conversation

xaptronic
Copy link
Contributor

@xaptronic xaptronic commented Sep 20, 2024

This PR adds support for running NiceGUI in enterprise mode.

Also adds a method to run grid methods with dynamic property conversion, so you can dynamically add / change callbacks and other things using properties with : prefix.

I am not overly familiar with this method of passing arguments into the class definition (for __init_subclass__). I imagine it might be preferred to be able to conditionally include the enterprise js package?


Feature request: #2431

@falkoschindler
Copy link
Contributor

Thanks for the pull request, @xaptronic!
Unfortunately, we have several concerns about including AG Grid Enterprise in NiceGUI:

  • The enterprise version adds 800kB of payload for everyone using the ui.aggrid element, even when working without a license key. This is quite a lot.
  • The NiceGUI package grows by 8.7MB, which is also pretty significant.
  • It's unclear if we're legally allowed to include these files in an MIT-licensed library.
  • Maintaining this feature without owning a license key ourselves will be rather challenging.

Therefore we think it might be better to create a separate, independently maintained ui.aggrid_enterprise element in a separate package and repository like we did with NiceGUI Highcharts. This way it can be installed when needed, reducing package size and payload for community users that don't need enterprise features.

Regarding the conversion of dynamic arguments:
Do we really need to introduce a new method? Can't we convert arguments by default?
Apart from that this feature should be split into a separate pull request, so that we can discuss it independently.

Long story short, I'll have to close this PR. But let's continue the discussion over at #2431 or on a new feature/pull request regarding the argument conversion. 🙂

@xaptronic
Copy link
Contributor Author

Fair enough - I'll look into creating an separate module for enterprise.

I will split out the argument conversion into another PR.

Can't we convert arguments by default?

I thought perhaps yes, but never know about unintended consequences.

@python-and-fiction
Copy link
Contributor

@xaptronic I suggest that you should add a classmethod to set license key. In order that we don't need to set it everytime when we create a enterprise aggrid table.

falkoschindler pushed a commit that referenced this pull request Oct 14, 2024
…method (#3866)

Following up with #3757

Do dynamic property conversion when using `run_grid_method` /
`run_row_method`, so you can dynamically add / change callbacks and
other things using properties with `:` prefix.
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.

3 participants