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.

Sign up for GitHub

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

Jump to bottom

docker: Add helper recipe #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

docker: Add helper recipe #31

wants to merge 1 commit into from

Conversation

rzr
Copy link
Contributor

@rzr rzr commented Jan 15, 2022

My distro RUST setup is not running it out of the box
so I used a more recent one.

For the record here is is the observed error:

error[E0599]: no variant or associated item named OutOfMemory found for enum ErrorKind in the current scope
-> /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/sqlx-core-0.5.10/src/sqlite/connection/establish.rs:107:32

Relate-to: #20
Signed-off-by: Philippe Coval philippe.coval@astrolabe.coop

rzr added a commit to CrossStream/astarte-device-sdk-rust that referenced this pull request Jan 15, 2022
@rzr
docker: Add helper recipe
de26f19 
My distro RUST setup is not running it out of the box
so I used a more recent one.

For the record the observed error was:

  error[E0599]: no variant or associated item named `OutOfMemory` found for enum `ErrorKind` in the current scope
  -> /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/sqlx-core-0.5.10/src/sqlite/connection/establish.rs:107:32

Forwarded: astarte-platform#31
Relate-to: astarte-platform#20
Signed-off-by: Philippe Coval <philippe.coval@astrolabe.coop>
@rzr rzr force-pushed the sandbox/rzr/review/master branch from 10b96cc to de26f19 Compare January 15, 2022 14:23
@shikoku1
Copy link
Contributor

My distro RUST setup is not running it out of the box

For development you should consider using rustup instead of the distribution's packaged compiler, as recommended by the rust project
Similarly, it might be better for the container to be based on the official rust image https://hub.docker.com/_/rust

Also, the docker build should be added to the CI if we want to support this

@rzr
Copy link
Contributor Author

rzr commented Jan 18, 2022

ok I thought about this too

rzr added a commit to CrossStream/astarte-device-sdk-rust that referenced this pull request Jan 18, 2022
@rzr
docker: Add helper recipe
7d44d79 
Using officla rust's docker image as base.

For the record it can be rebased on ubuntu:21.10
(and cargo should be added to base packages)

My distro RUST setup is not running it out of the box
so I used a more recent one.

For the record the observed error was:

  error[E0599]: no variant or associated item named `OutOfMemory` found for enum `ErrorKind` in the current scope
  -> /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/sqlx-core-0.5.10/src/sqlite/connection/establish.rs:107:32

Forwarded: astarte-platform#31
Relate-to: astarte-platform#20
Signed-off-by: Philippe Coval <philippe.coval@astrolabe.coop>
@rzr rzr force-pushed the sandbox/rzr/review/master branch from de26f19 to 7d44d79 Compare January 18, 2022 14:27
@rzr
Copy link
Contributor Author

rzr commented Jan 21, 2022

"the docker build should be added to the CI if we want to support this" this can be done in next patch using github actions ?

shikoku1
shikoku1 reviewed Jan 24, 2022
View reviewed changes
Dockerfile Outdated
Comment on lines 37 to 38
&& cargo install --path . \
|| cargo install --examples --path . \
Copy link
Contributor

Choose a reason for hiding this comment

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

is this really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me double check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it's needed I have updated the commit message

shikoku1
shikoku1 reviewed Jan 24, 2022
View reviewed changes
Dockerfile Outdated

WORKDIR "${workdir}"
ENTRYPOINT [ "cargo" ]
CMD [ "run", "--example", "simple", "--help" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what the expected usage is, feel free to add instructions into the readme

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the default run command , it could be overridden when using docker run ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah but now it's calling --help for cargo, overriding --example, it doesn't do anything as it is. I can get what the idea was but I think we should fix this and document the intended usage anyway, because a dockerfile can be use in different ways (for example to produce binaries in a volume)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--help for example will be displayed not cargo

shikoku1
shikoku1 reviewed Jan 24, 2022
View reviewed changes
Dockerfile Outdated
@@ -0,0 +1,43 @@
#!/bin/echo docker build . -f
# -*- coding: utf-8 -*-
# SPDX-License-Identifier: MPL-2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

The rest of the project is licensed under Apache 2.0, is it ok to have a single file under a different license?
CC: @bettio

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mistake let me align

rzr added a commit to CrossStream/astarte-device-sdk-rust that referenced this pull request Jan 24, 2022
@rzr
docker: Add helper recipe
c7a87d9 
Using official rust's docker image as base.

For the record it can be rebased on ubuntu:21.10
(and cargo should be added to base packages)

My distro RUST setup is not running it out of the box
so I used a more recent one.

For the record the observed error was:

  error[E0599]: no variant or associated item named `OutOfMemory` found for enum `ErrorKind` in the current scope
  -> /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/sqlx-core-0.5.10/src/sqlite/connection/establish.rs:107:32

Install example is needed because there is no default installation rule:

  error: there is nothing to install in `astarte_sdk v0.1.0 (/local/src/astarte-device-sdk-rust)`, because it has no binaries

Forwarded: astarte-platform#31
Relate-to: astarte-platform#20
Signed-off-by: Philippe Coval <philippe.coval@astrolabe.coop>
@rzr rzr force-pushed the sandbox/rzr/review/master branch from 7d44d79 to c7a87d9 Compare January 24, 2022 18:31
rzr added a commit to CrossStream/astarte-device-sdk-rust that referenced this pull request Feb 28, 2022
@rzr
docker: Add helper recipe
2b09b12 
Using official rust's docker image as base.

Usage is straight forward:

```
    docker run astarte-device-sdk-rust:latest run --example simple -- \
	   --credentials-secret $credentialssecret \
	   --device-id $deviceid \
	   --pairing-url $pairingurl \
	   --realm $realm
```

For the record it can be rebased on ubuntu:21.10
(and cargo should be added to base packages)

My distro RUST setup is not running it out of the box
so I used a more recent one.

For the record the observed error was:

  error[E0599]: no variant or associated item named `OutOfMemory` found for enum `ErrorKind` in the current scope
  -> /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/sqlx-core-0.5.10/src/sqlite/connection/establish.rs:107:32

Install example is needed because there is no default installation rule:

  error: there is nothing to install in `astarte_sdk v0.1.0 (/local/src/astarte-device-sdk-rust)`, because it has no binaries

Forwarded: astarte-platform#31
Relate-to: astarte-platform#20
Signed-off-by: Philippe Coval <philippe.coval@astrolabe.coop>
@rzr
docker: Add helper recipe
c184760 
Using official rust's docker image as base.

Usage is straight forward:

```
    docker run astarte-device-sdk-rust:latest run --example simple -- \
	   --credentials-secret $credentialssecret \
	   --device-id $deviceid \
	   --pairing-url $pairingurl \
	   --realm $realm
```

For the record it can be rebased on ubuntu:21.10
(and cargo should be added to base packages)

My distro RUST setup is not running it out of the box
so I used a more recent one.

For the record the observed error was:

  error[E0599]: no variant or associated item named `OutOfMemory` found for enum `ErrorKind` in the current scope
  -> /root/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/sqlx-core-0.5.10/src/sqlite/connection/establish.rs:107:32

Install example is needed because there is no default installation rule:

  error: there is nothing to install in `astarte_sdk v0.1.0 (/local/src/astarte-device-sdk-rust)`, because it has no binaries

Forwarded: astarte-platform#31
Relate-to: astarte-platform#20
Signed-off-by: Philippe Coval <philippe.coval@astrolabe.coop>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shikoku1 shikoku1 shikoku1 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rzr @shikoku1