Skip to content

Conversation

seinecle
Copy link

@seinecle seinecle commented Sep 1, 2022

New plugin or plugin update?

  • New Plugin
  • Update

What is the purpose of this plugin?

Enable the user to publish the network they are working on, to the web. See this issue for details of the implementation:
#262 (comment)

How to test your plugin in Gephi?

  1. Go to File -> Export -> Publish to the Web
  2. Follow the instructions in the "Setup" tab
  3. Switch to the "Publish" tab and publish.

Checklist before submission

  • Did you merged with the master branch to get the latest updates?
  • Did you build and test the plugin successfully locally?
  • Did you include metadata (e.g. plugin author, license) in the pom.xml file
  • Did you make sure NOT to include any unecessary files in your PR?
  • Did you write unit tests to test your plugin functionalities?

@mbastian mbastian changed the title Initial commit Web publish plugin Sep 1, 2022
@mbastian mbastian linked an issue Sep 2, 2022 that may be closed by this pull request
@mbastian mbastian self-requested a review September 13, 2022 10:14
@mbastian
Copy link
Member

Nice @seinecle and sorry for the delay. I started looking into it and have some general comments and questions:

  1. You currently don't configure the GEXF exporter. Are all the default settings what you would expect? For instance I don't think the viewer support dynamic graphs so we should probably turn this off.
  2. How do you deal with Gist's limits? Seems like the limit could be reach fairly quickly with large graphs so we should probably deal with it. At least inform the user. Maybe we should also thing about strategies to reduce the file size? Maybe Sigma.js supports zipped's GEXF?
  3. You have a number of hardcoded strings in your code like URLs, clientId etc. Could you turn all of those into constants for code clarity? (private static final String)

@seinecle
Copy link
Author

hi @mbastian,

  1. I didn't know the exporter can be configured. I feel like the export to gist should preserve the network exactly as the user is working on it in Gephi, so I would not restrain / limit the gexf to some features. That's rather up to Retina / Sigma to handle it in the way they need?
  2. I was not aware of these limits. I'll have a look and yes, I will add warnings and will add ways to handle the exceptions related to limits
  3. Good point, will do.

@seinecle
Copy link
Author

seinecle commented Sep 19, 2022

Did some verifications:

Gist has a push limit at 100Mb per file (when not using GLFS).

I used the plugin to publish a network which corresponds to a gexf file of 61Mb with 2000 nodes and 279,000 edges.

  • the upload on Gist works fine, as expected
  • Retina can't display it: no error msg, the page doesn't load

During the Gephi week I had asked @jacomyal about rough upper limits for the size of a network to be displayed by Retina. It would be around 10k nodes and 20k edges - above this, the browser might get slower on some computers.

So it seems that the first limit is the network size that Retina can handle, not the max file size Gist accepts.

I propose that we set a limit to the size of the network to be published? Like: 20k nodes max, 40k edges max? If any of these limits is reached, the "Publish" button remains greyed out with an explainer.

Your comments are welcome @mbastian @jacomyal

@mbastian
Copy link
Member

@seinecle Thanks for verifying! It's a good news although it does contradict the Gist documentation, which I find strange. The link you posted is for GitHub and may not apply to Gist.

Regarding the size limit I'm not a fan of forbidding the upload but maybe rather inform the user that the experience is not gonna be optimal. The node/edge counts is also only partially a factor in the file size. The number of attributes and their content can also be a bit factor.

@seinecle
Copy link
Author

Ok, so no hard limit but a warning then - I'll update the plugin today.

seinecle and others added 3 commits September 20, 2022 16:16
…iting to cancel the publication. Added a limit at 100Mb for the size of networks to be uploaded, to prevent an error from Github.
Copy link
Member

@mbastian mbastian left a comment

Choose a reason for hiding this comment

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

I added some comments, mostly related to conventions. One general comment about the architecture is trying to better decouple the business logic from the UI. You have part of it in the PublishingActions, which I think is great as it allows testability. You can check the 3 unit tests I created for reference. On the other hand, some other logic is buried somewhere in the JPanelWebExport class, making it difficult to unit test. Ideally all logic is wrapped into clear functions that can be unit tested.

}
} catch (IOException | InterruptedException ex) {
Exceptions.printStackTrace(ex);
jTextFieldGithubErrorMsg.setText(ex.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

Within the SwingWorker class you're running on a separate thread than the EDT (the swing thread). All the Swing code should normally be sent back to the EDT, usually with SwingUtilities.invokeLater(). It will work but not fully correct.

Copy link
Author

Choose a reason for hiding this comment

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

I have moved the plugin logic outside of the JPanel. But for the SwingWorker which polls Github, I still don't get how the result of the worker can be sent back to the JPanel in a clean way, once it completes.

The way I do it is that the done() method of the GithubPollerCreator class sets the text of a jTextFieldGithubErrorMsg, a variable situated in the JPanel. I do that by making the TextField a static variable in the JPanel, so that it can be accessed in the done() method. See here:

0530df9#diff-daad6c7d4cdd5ffbe1662e9768aa94858a1f0776311d95cae1acc5d0f3a9b80eR80

But it doesn't feel optimal. I'd be curious to learn how to do it in a correct way.

@seinecle
Copy link
Author

Wow indeed many improvements to make. My comments:

  • on the json object that I use to pass stuff around. It is indeed weird and not ideal. You suggest that I could throw exceptions instead. Ok, good! I have never worked this way though, so I should get back to some Java core textbook. Will take a couple of days.
  • on the SwingWorker / EDT stuff: will go and try what you suggest. I am not familiar at all with these things so any code example / resource would be great, by any chance.

@mbastian
Copy link
Member

@seinecle I did a deeper refactoring to help decouple the UI from the business logic, which was still tightly coupled. In addition, I uses our LongTask API instead of SwingWorker. With that, you easily get the progress bar too. The two Runnable classes are now completely decoupled from the UI. The UI updates itself through a LongTaskListener (success) and LongTaskErrorHandler (error).

One UI improvement I think you could still make is on the final result. It's really paintful to have to copy-paste the final URL from a text field. Why not having an "Open" button or "Copy to clipboard"? Java can easily open the browser for you with an URL.

… the browser. Localized remaining strings. Renamed a class
@seinecle
Copy link
Author

@mbastian thanks! LongTask seems super nice, glad to discover this utility! And thanks for the refactoring.

I have:

  • added the buttons you mention (open in web browser + copy link to clipboard).
  • this needed to change slightly the way the LongTask was returning strings
  • localized all strings
  • renamed a class to WebExport JPanel

@mbastian
Copy link
Member

Thanks @seinecle! I just noticed that the access token we get from GitHub expires after 8 hours. This isn't something we can change but there is a flow we can use via refresh tokens to have 6 months tokens. It seems unpractical to have to go through the authentification every day so it would be good to implement that. We shall also change the narrative in the UI. It's a little more complicated than "Setup once".

@seinecle
Copy link
Author

ok, thanks for identifying this! Will have a look.

@seinecle
Copy link
Author

@mbastian reading the doc, it seems we can just opt out of expiring tokens? If I understand correctly, the 8 hours limit is just for expiring tokens.
So in Gephi's github app ("gephi-lite"), there just needs to opt out expiring tokens. The steps to find this option are detailed in the link you included in your comment. Please let me know if I misunderstand something.

… design: added borders to the 2 panels in the publication tab.
@mbastian
Copy link
Member

@seinecle Interesting. It still don't really get from the doc what happens when we opt-out? Access token don't expire at all? I opted out now. Let's see how it goes

@seinecle
Copy link
Author

@mbastian do you think we can move to the publication?

@mbastian
Copy link
Member

@seinecle Yes, almost :) Looks like you inverted the buttons between gist view versus network viz.

@mbastian mbastian changed the base branch from master to master-forge September 30, 2022 17:18
@mbastian mbastian merged commit 398a8ff into master-forge Sep 30, 2022
@mbastian
Copy link
Member

@seinecle The plugin is published https://gephi.org/plugins/#/plugin/web-publish-plugin

Thanks for this great work! cc @jacomyal

@seinecle
Copy link
Author

seinecle commented Oct 1, 2022

@mbastian Thank you for the extensive code review !!

@DeJeroenBakker
Copy link

First of all, thanks for developing this plugin @seinecle. I can imagine this being very useful!

The plugin doesn't seem to work for me at the moment, however. The authentication process works flawlessly, but when I try to publish my network, I always get the following 500 error:

Error creating online gexf file. Error code:500; Error message:

Is there any way to fix this? I've tried re-authenticating, using a different network and restarting Gephi, but it doesn't seem to make a difference.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Publish to the Web plugin

3 participants