Skip to content

Conversation

panisson
Copy link
Member

This is an update of the existing plugin for version 0.8.2. This first port of the plugin provides a quick solution for those looking for the same functionality available in 0.8.2.

The following functionalities have not yet been updated, but they will be updated once the proper API is available:

  • Generate streaming events when attribute values are changed.

build.xml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This file isn't necessary and can be deleted. It's a legacy file from the Ant-based plugin.

Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify if that's actually possible? I'm not sure in which scenario you would have to create the project. I'm asking because that isn't the right way to create a project when their a GUI (it is if done from toolkit for instance) and one should rather use the ProjectControllerUI.newProject() in desktop-project module instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I already addressed this, not sure why the comment is not showing up as "outdated"...

Copy link
Member

Choose a reason for hiding this comment

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

You don't need to wrap a addEdge call with a writeLock as it's done for you under the hood. However it could make sense to extend the write lock to the entire block. I can see an issue if the source or target node gets removed between the time you query them and the time you create and add the edge.

Copy link
Member

Choose a reason for hiding this comment

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

Remove write lock

@mbastian
Copy link
Member

Some additional comments as the diff is a little long for GitHub:

What is this file? modules/StreamingImpl/src/main/resources/org/gephi/streaming/impl/cacerts

The file modules/StreamingImpl/src/main/resources/org/gephi/streaming/impl/layer.xml can probably be deleted, it’s empty.

Looks like the modules/StreamingServer/pom.xml repeats the Jetty dependencies, can this be replaced by a dependency on JettyWrapper?

In modules/StreamingServer/src/main/java/org/gephi/streaming/server/impl/GraphWriter.java, maybe use column.getId() instead of getTitle()?

What is this file? modules/StreamingServer/src/main/resources/org/gephi/streaming/server/impl/localhost.p12

Also delete modules/StreamingServer/src/main/resources/org/gephi/streaming/server/layer.xml?

@mbastian mbastian self-assigned this Jan 27, 2016
@jeromatron
Copy link

Just curious - is this likely to make it in soon? It's a very useful plugin, but it would be great to have it work with the current version.

@panisson
Copy link
Member Author

@mbastian: I did try to address most of your comments, thanks for the review!

About the file modules/StreamingServer/src/main/resources/org/gephi/streaming/server/impl/localhost.p12, this is an encryption key for ssl in the StreamingServer. It's not the perfect way for security, but it should just add support for https in the StreamingServer. Anyone worried about security in this case should point to his own encryption key.

@mbastian
Copy link
Member

Cool! Looks good to me. Last request, can you merge from master and update your module poms to 0.9.1:

<parent>
        <artifactId>gephi-plugin-parent</artifactId>
        <groupId>org.gephi</groupId>
        <version>0.9.1</version>
    </parent>

@panisson
Copy link
Member Author

That's all, it should be ready for 0.9.1 🎆

@mbastian mbastian merged commit a80c3ea into gephi:master-forge Feb 16, 2016
mbastian added a commit that referenced this pull request Feb 16, 2016
@mbastian
Copy link
Member

Congrats! Your plugin is now available in Gephi and online at https://gephi.org/plugins/#/plugin/graphstreaming

eduramiba pushed a commit to eduramiba/gephi-plugins that referenced this pull request Feb 27, 2017
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.

5 participants