Skip to content

Conversation

vitvakatu
Copy link
Contributor

@vitvakatu vitvakatu commented Oct 18, 2019

Overview

Unfortunately we cannot (yet) test this plugin, but given extremely simple implementation, it probably should work. We will be able to test it after implementing Java Runtime.


See: https://jira.bf.local/browse/ECR-3692

Definition of Done

  • There are no TODOs left in the code
  • Change is covered by automated tests
  • The coding guidelines are followed
  • Public API has Javadoc
  • Method preconditions are checked and documented in the Javadoc of the method
  • Changelog is updated if needed (in case of notable or breaking changes)
  • The continuous integration build passes

@coveralls
Copy link

coveralls commented Oct 18, 2019

Coverage Status

Coverage increased (+0.9%) to 87.005% when pulling 5a9801f on vitvakatu:launcher_runtime_plugin into fad3c49 on exonum:dynamic-services.

artifact: {}
```

To load service artifact, provide a path to the service artifact, for example:
Copy link
Contributor

Choose a reason for hiding this comment

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

not a path, but a file name — and please specify where the file shall be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed, added todo to move this entire section to ejb app tutorial. This readme is not intended to be used by our end-users.

raise RuntimeError("Use proto/generate_proto.sh script to generate protobuf definition")


class JavaRuntimeSpecLoader(RuntimeSpecLoader):
Copy link
Contributor

Choose a reason for hiding this comment

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

DeploySpec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tend to avoid extremely long names, as the users will need to use them in their config files, so I renamed it to JavaDeploySpecLoader

protoc = find_executable('protoc')


def generate_proto(source):
Copy link
Contributor

Choose a reason for hiding this comment

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

I very much would not like to have this code in the project which does things that a build system must do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, it is the only possible option here and now. The only project I found that does the same thing (https://github.com/pupssman/protobuf-setuptools) does not support integration into our own setup.py. It's weird, but I wasn't able to find a solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can configure the build to generate Python sources (either by adding an execution to core module, or to a new module). Would that work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to maven configuration of the core module

- [`exonum-launcher`](https://github.com/exonum/exonum-launcher)

```bash
cd exonum-java-binding
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably cd is irrelevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted to show current working directory. Probably yes, removed

#### Requirements

- [Python3](https://www.python.org/downloads/)
- [`exonum-python-client`](https://github.com/exonum/exonum-python-client)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do I have to install these manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now yes, later we will provide an easier approach (after publishing on pypi)

@dmitry-timofeev dmitry-timofeev merged commit a8b8209 into exonum:dynamic-services Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants