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

Add Login Action #1

Merged
merged 1 commit into from
Apr 2, 2019
Merged

Add Login Action #1

merged 1 commit into from
Apr 2, 2019

Conversation

Greg-Hamel
Copy link
Contributor

@byCedric,

Thanks a lot for putting efforts in trying to get actions available for the expo-cli. I've been looking at getting those implemented, but lacked the motivation/knowledge to get started.

During my tests of your action, I've been having lots of problems getting a successful and censored login with the expo-cli. I'm sure you are still working on it as I can see that your docker hub has a few recent changes.

I've come to realise that in order to use the SECRETs without them returning 'Filtered', they need to be use by the entrypoint.sh directly and not through arguments. I was inspired by the how the docker/login action works.

The login action in this PR seems to work flawlessly in the different tests I've made, even censoring the SECRET correctly.

Let me know what you think!

Cheers,

@byCedric
Copy link
Member

byCedric commented Apr 2, 2019

Hi @Greg-Hamel! Thanks for the feature, it was in the "backlog of tasks" for me too 😉. Unfortunately, I ran into some issues when building new versions from Action. It's related to the maximum inotiy listeners allowed on the Actions' system. I contacted Github to see if we could work this out. My last reply from them is this:

There isn't an option that I'm aware of. However, I did field this case and all of the context you shared with us (really helpful details, thanks a ton for that!) internally for our Actions team to review. Because the feature is still in Beta, we are considering all of the input we get before taking on new initiatives. We don't expect that the limits you have observed will change any time soon, but we recommend following the GitHub Actions changelog for the latest updates:

https://developer.github.com/actions/changes/

We're sorry we don't have another workaround for this though we'll be sure to get in touch if we hear more from the team.

(Did some research why it worked on other vendors, despite having the same inotify)

So unfortunately, until this is resolved either by Expo or GitHub Actions, the most crucial command (publish) is blocked. I'm at the conference April 4th, I can try to bring it to their attention.

Again, thanks for this, really appreciate it ❤️

@byCedric byCedric merged commit 252f16d into expo:master Apr 2, 2019
@byCedric
Copy link
Member

byCedric commented Apr 2, 2019

I'll add some other files to keep the sturcture in sync with cli 😄 (just the basic stuff like license etc)

@Greg-Hamel
Copy link
Contributor Author

@byCedric,

Thanks for the quick merge, as for the problem with inotify, I haven't encountered the same issue... yet. Maybe our Actions are a tad different, mine is still in its early stages (not a whole lot of actions yet).

My publish seems to work correctly, and I feel like I took it directly from one of your test repos.

Screenshot 2019-04-02 at 09 50 31

Here is the log from the publish part:

Screenshot 2019-04-02 at 09 53 35

And of course, the main.workflow

workflow "Build and publish on Expo" {
  on = "push"
  resolves = [
    "Master Branch Only",
    "Publish to Expo",
  ]
}

action "Build" {
  uses = "actions/npm@master"
  args = "install"
}

action "Test" {
  needs = ["Build"]
  uses = "actions/npm@master"
  args = "test"
}

action "Master Branch Only" {
  uses = "actions/bin/filter@master"
  args = "branch master"
  needs = ["Test"]
}

action "Install dependencies" {
  uses = "bycedric/ci-expo/cli@master"
  runs = "npm"
  args = "ci"
  needs = ["Master Branch Only"]
}

action "Login with Expo" {
  uses = "Greg-Hamel/ci-expo/login@master"
  secrets = ["EXPO_USERNAME", "EXPO_PASSWORD"]
}

action "Publish to Expo" {
  uses = "docker://bycedric/ci-expo"
  needs = ["Login with Expo", "Install dependencies"]
  secrets = ["EXPO_USERNAME", "EXPO_PASSWORD"]
  runs = "expo publish"
}

@byCedric
Copy link
Member

byCedric commented Apr 2, 2019

Wow! That looks awesome! Let me try to set up a basic project and see what happens. It might be fixed already then! 😄 Thanks for this!

@byCedric
Copy link
Member

byCedric commented Apr 2, 2019

Dude, you are right! It's working now without the max inotify reached issue! I'll finish up some work and create a first stable release for this tonight, just in time for the Expo conference 😄 Thanks a lot ❤️

@Greg-Hamel
Copy link
Contributor Author

@byCedric, my pleasure! Glad I could help you help me!

Keep up the great work 🏆 🎉

@byCedric
Copy link
Member

byCedric commented Apr 2, 2019

You too @Greg-Hamel 🚀!

I just updated the master, consider this as a pre-release. I'm going to add the 3 build commands as separate actions, and then it's ready.

One change to notice is that I've moved the cli action to root. That means you have to change bycedric/ci-expo/cli@master to bycedric/ci-expo@master. I did document it here, but wanted to give you a heads up.

@mgibeau
Copy link

mgibeau commented Apr 17, 2019

We ended up actually hitting the inotify limit in the last day or so.

First thing we did was try to change it within the Docker context. This doesn't seem to be supported at the moment, and as the comment from GitHub suggest might not be for a while... You can see the attempted fix: master...mgibeau:master. This still returns 8192 even after attempting to change it.

What fixed it (for now) though, is to add an extra step during the pipeline to prune the devDependencies from the build (since they are not required by expo). My understanding is that expo is scanning the whole node_modules folder when building so removing some of them allows us to publish successfully:

# ... some actions to install, test, etc ...

action "Cleanup" {
  needs = ["Release"]
  uses = "docker://node:alpine"
  args = "yarn install --production"
}

# expo publish *must* depend on cleanup
action "Publish" {
  needs = ["Cleanup"]
  uses = "expo/expo-github-action@2.3.1"
  args = "publish"
  secrets = ["EXPO_CLI_USERNAME", "EXPO_CLI_PASSWORD"]
}

Hopefully, this might help someone else who stumbles upon that problem :)

@byCedric
Copy link
Member

@mgibeau Thanks a lot! That's something I didn't know for sure 😄 I have been in contact with the GitHub support team, they said they made a case for the developers. But they couldn't provide any ETA or status update, if and when the inotify is getting fixed. There are some other actions hitting this limit too, unfortunately. But I think this is a great workaround, thanks! ❤️

byCedric pushed a commit that referenced this pull request Dec 4, 2020
…62)

* Now uses official @actions/cache package

* Run yarn upgrade

* Add handling for ReserveCacheError (#1)

Bump @actions/cache from 1.0.2 to 1.0.3
Add missing caret to specify @actions/cache version range
Add handling for ReserveCacheError
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.

3 participants