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

Remove non working analyze explain button #17995

Merged
merged 1 commit into from Jan 17, 2023

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Dec 28, 2022

Description

The service mariadb.org/explain_analyzer is unavailable.

I asked MariaDB via Slack and it's broken for a while.

Fixes: #17030

Before submitting pull request, please review the following checklist:

  • Make sure you have read our CONTRIBUTING.md document.
  • Make sure you are making a pull request against the correct branch. For example, for bug fixes in a released version use the corresponding QA branch and for new features use the master branch. If you have a doubt, you can ask as a comment in the bug report or on the mailing list.
  • Every commit has proper Signed-off-by line as described in our DCO. This ensures that the work you're submitting is your own creation.
  • Every commit has a descriptive commit message.
  • Every commit is needed on its own, if you have just minor fixes to previous commits, you can squash them.
  • Any new functionality is covered by tests.

@williamdes
Copy link
Member

Can you ask them if they can open source it?

@codecov
Copy link

codecov bot commented Dec 29, 2022

Codecov Report

Base: 47.22% // Head: 47.72% // Increases project coverage by +0.49% 🎉

Coverage data is based on head (79eacd6) compared to base (f7d9321).
Patch coverage: 44.44% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             QA_5_2   #17995      +/-   ##
============================================
+ Coverage     47.22%   47.72%   +0.49%     
- Complexity    16825    16959     +134     
============================================
  Files           602      607       +5     
  Lines         71576    72301     +725     
============================================
+ Hits          33805    34508     +703     
- Misses        37771    37793      +22     
Flag Coverage Δ
dbase-extension ?
recode-extension 47.14% <43.96%> (-0.06%) ⬇️
unit-7.2-ubuntu-latest ?
unit-7.3-ubuntu-latest 48.93% <47.03%> (+1.25%) ⬆️
unit-7.4-ubuntu-latest 47.65% <47.03%> (-0.07%) ⬇️
unit-8.0-ubuntu-latest 47.73% <47.03%> (-0.74%) ⬇️
unit-8.1-ubuntu-latest 47.77% <47.03%> (+0.09%) ⬆️
unit-8.2-ubuntu-latest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
libraries/classes/Html/Generator.php 24.59% <ø> (+0.51%) ⬆️
libraries/classes/Plugins/TwoFactor/Key.php 17.97% <0.00%> (ø)
libraries/classes/WebAuthn/CustomServer.php 0.00% <0.00%> (ø)
libraries/classes/WebAuthn/WebauthnLibServer.php 44.23% <44.23%> (ø)
libraries/classes/WebAuthn/DataStream.php 59.09% <59.09%> (ø)
libraries/classes/WebAuthn/CBORDecoder.php 94.30% <94.30%> (ø)
...asses/Controllers/JavaScriptMessagesController.php 99.63% <100.00%> (+<0.01%) ⬆️
libraries/classes/Plugins/TwoFactor/WebAuthn.php 100.00% <100.00%> (ø)
libraries/classes/TwoFactor.php 77.06% <100.00%> (+0.21%) ⬆️
libraries/classes/Plugins/Import/ImportShp.php 58.10% <0.00%> (-14.19%) ⬇️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kesselb
Copy link
Contributor Author

kesselb commented Jan 4, 2023

Can you ask them if they can open source it?

Sure

@williamdes williamdes added this to the 5.2.2 milestone Jan 8, 2023
@williamdes
Copy link
Member

This would close: #17030

If they agree to open source it we can maybe make it internal.

@MauricioFauth since it's broken it is okay to drop this link ?
Or should we disable it with a tooltip for users ?

@MauricioFauth
Copy link
Member

@MauricioFauth since it's broken it is okay to drop this link ? Or should we disable it with a tooltip for users ?

I think the button should be removed.

@LinuxJedi
Copy link

Hi all,

I am the Chief Contributions Officer for MariaDB Foundation.

Sorry for the delay in responding to this. I have been on a long vacation over Christmas and New Year and have had to do a bit of research into the history of this tool.

Unfortunately we cannot open source it in its current form. Due to a complicated history (that pre-dates the Foundation itself) we do not have the rights to do so and have been unsuccessful in obtaining them. That being said we at the Foundation do have access to the source code (currently written in Python 2) and do want to reverse engineer it into a new open source tool.

We have not decided on a language or what form this will take yet. We are thinking some kind of library / cli with a JSON output. But are willing to work with the phpMyAdmin community on this if it is something useful.

I'm more than happy to discuss this further as needed.

@williamdes
Copy link
Member

Hi all,

Hi
It's an honor to have your feedback !

Unfortunately we cannot open source it in its current form. Due to a complicated history (that pre-dates the Foundation itself) we do not have the rights to do so and have been unsuccessful in obtaining them. That being said we at the Foundation do have access to the source code (currently written in Python 2) and do want to reverse engineer it into a new open source tool.

Oh okay, thank you for making this clear.

We have not decided on a language or what form this will take yet. We are thinking some kind of library / cli with a JSON output. But are willing to work with the phpMyAdmin community on this if it is something useful.

I'm more than happy to discuss this further as needed.

We would be delighted to discuss about this and follow this new project. If we could embed the tool in phpMyAdmin I am sure the users would like it very much !

External services would lead to data leaks like it is reported in #17030

@williamdes williamdes changed the base branch from master to QA_5_2 January 17, 2023 13:12
The service mariadb.org/explain_analyzer is unavailable.

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@williamdes williamdes modified the milestones: 5.2.2, 5.2.1 Jan 17, 2023
@williamdes williamdes self-assigned this Jan 17, 2023
@phpmyadmin phpmyadmin deleted a comment from phpmyadmin-bot Jan 17, 2023
@phpmyadmin phpmyadmin deleted a comment from phpmyadmin-bot Jan 17, 2023
@phpmyadmin phpmyadmin deleted a comment from phpmyadmin-bot Jan 17, 2023
@phpmyadmin phpmyadmin deleted a comment from phpmyadmin-bot Jan 17, 2023
@phpmyadmin phpmyadmin deleted a comment from phpmyadmin-bot Jan 17, 2023
@williamdes williamdes changed the title fix: remove analyze explain button Remove non working analyze explain button Jan 17, 2023
williamdes added a commit that referenced this pull request Jan 17, 2023
Ref: f1581cf
Ref: #17030
Pull-request: #17995

Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes williamdes merged commit f1581cf into phpmyadmin:QA_5_2 Jan 17, 2023
24 of 30 checks passed
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

None yet

4 participants